Closed lwasser closed 6 months ago
@lwasser Can you please provide a brief note for the PR on what changes are here ☀️
@willingc done!! i also have a question about organization.
Attention: Patch coverage is 73.05699%
with 52 lines
in your changes are missing coverage. Please review.
Project coverage is 46.49%. Comparing base (
9f61c17
) to head (22496cb
).:exclamation: Current head 22496cb differs from pull request most recent head dcd9460. Consider uploading reports for the commit dcd9460 to get more accurate results
Files | Patch % | Lines |
---|---|---|
src/pyosmeta/models.py | 71.69% | 42 Missing and 3 partials :warning: |
src/pyosmeta/clean.py | 73.07% | 7 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@lwasser Let's do this. Let me review this PR as is. We can then tackle simplifying the GitHub token use and naming in separate iterative PRs. Each PR can be small and tested with the goal of keeping the functionality working with minimal changes at once 👍🏼
@willingc that would be wonderful. i really appreciate your reviewing!! 🙌
i really see the value of smaller pr's. this one feels big big BUT it's mostly my moving things around and renaming. (and i removed some comments from my prior less create an issue savy self)
This pr is an attempt to clean up and reorganize the various elements of this package. it does the following.
I have a question however. Right now #3 above - both modules need to call the github API. And as such i share the token authentation between the two classes. I wondered if it might make more sense to create a new github module that just handles github things? and i'd love some feedback on this.
The goal here is to make the code easier to maintain and read.