Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add: Review model should include labels (OR is there a better way to access labels)? #188

Closed
lwasser opened this issue Jul 12, 2024 · 3 comments · Fixed by #189
Closed

Comments

@lwasser
Copy link
Member

lwasser commented Jul 12, 2024

Current we have a peer-review-metrics repo that supports gathering information on our current and past review status. The goal is to turn this into a dashboard for editors and the EiC to understand the current state of reviews while also collecting review metrics that I / we can use for reporting.

The current refactor is great but it broke a few things over in peer-review metrics (built with myst).

see this notebook as an example. One of the things that we want to access with our review issues is the labels. The labels tell us the status of a review, and we need that information to create our dashboard of current and previous reviews.

we also want to collect metadata on the issue such as date opened, date closed, etc.

I wonder if we create a separate object or add a subclass to ReviewModel that stores Issue metadata including

  • labels
  • date_opened
  • date_closed

We may want to add to this in the future as well.
@sneakers-the-rat what do you think makes sense structurally here for this?

Would something like

ReviewModel:
    issue_meta: IssueMeta
    
IssueMeta:
     labels: list[str]
     date_opened: datetime 
     date_closed: datetime

Make sense ?

i wasn't thinking about our little dashboard that i've been playing with when we merged! but i think your structure is still much much cleaner!

make sense as a subclass that captures metadata like labels,

@sneakers-the-rat
Copy link
Contributor

sure, could definitely be done. we make the model how we want it to work! ideally we do any big changes like this before anyone is actually depending on the fine structure of it (idk how many people actually are, but if there are many then we would want some deprecation process).

we already have this one liner for directly copying fields from the Issue, so eg. we already have created_at and closed_at which come from the Issue model, i'll PR grabbing labels now.

semantically i guess all of the contents of the ReviewModel come from an issue so are technically IssueMeta, but if it would be helpful to group the fields that we are directly copying from the issue api response then by all means!

@lwasser
Copy link
Member Author

lwasser commented Jul 12, 2024

Thank you so much @sneakers-the-rat ! If it helps - right now this package supports:

  1. generating that yaml file that populates our website and
  2. That peer review metric "dashboard" i've been working on just as a side thing.

So those two builds are the only ones that our changes will impact given this is a specific-to-pyOpenSci package.

it's really nice to think this all through like you are doing. this project evolved from a small script i initially wrote that became increasingly more complex. the organizing that you are providing here is so so helpful!! thank you again!!

@sneakers-the-rat
Copy link
Contributor

You have enough to do running the org, I figure it would be nice to have this be a "just works" thing you dont have to worry about. Happy to help ♥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants