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

Refactor needed for parse_issue_header method #91

Closed
lwasser opened this issue Feb 22, 2024 · 0 comments · Fixed by #115
Closed

Refactor needed for parse_issue_header method #91

lwasser opened this issue Feb 22, 2024 · 0 comments · Fixed by #115
Labels
help wanted Extra attention is needed refactor

Comments

@lwasser
Copy link
Member

lwasser commented Feb 22, 2024

Phew - working on the code i'm noticing some VERY convoluted structures i have around parsing the issue metadata that make it very hard to follow the code. In the next PR where i manage partnership data, i have the todo below added to the code:

 # TODO: this method parse_issue_header is working but the parsing code is 
    # really hard to follow
    # It is worth another pr that cleans up the workflow around grabbing 
    # metadata so it's clearer to follow.  
    accepted_reviews = process_review.parse_issue_header(issues, 45)

we need to refactor this part of the workflow so

  1. the methods are clearly named and scoped to do specific things and it's clear what happens when
  2. the docstrings clarify what the methods do on top of the names of the methods being clear
  3. I suspsect refactoring would also mean some code cleanup in general as the method now returns a bunch of extra unprocessed data that doesn't need to be returned.

specifically in this part of the process_Reviews.py script

final_reviews = {}
    for key, review in all_reviews.items():
        # First add gh meta to each dict
        print("Parsing & validating", key)

the review data returned looks like the image below. Note that in the image below, there is a bunch of raw github issue data that hasn't been cleaned

like this line
'- [x] I have read and will commit to package maintenance after the review as per the [pyOpenSci Policies Guidelines][Commitment].'

and then there is some nicely formatted data which is what we want that method to return like this:

`date-accepted`: '02/06/2024'

that message uncleaned data above shouldn't be returned at all.

the code is working now because the pydantic model just ignores those elements. I think our methods should be more intentional about returning clean data. They shouldn't just work because it works.

Screenshot 2024-02-21 at 7 33 18 PM
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed refactor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant