georgetown-cset / funder-finder

Retrieve GitHub repo funding information
Apache License 2.0
7 stars 3 forks source link

Refactor finders into classes, add wrapper script #30

Closed jmelot closed 1 year ago

jmelot commented 1 year ago

Closes #24

@jspeed-meyers this isn't quite done, but before propagating these changes to the github sponsors scraper I wanted to check in about how you felt about the direction this is going. I added some (hopefully not too confusing/onerous) structure to the finders so they would have a consistent interface that could be called by the wrapper script.

github-actions[bot] commented 1 year ago

No need for rebasing :+1: behind_count is 0 ahead_count is 6

github-actions[bot] commented 1 year ago

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
342 230 67% 0% 🟢

New Files

File Coverage Status
funderfinder/sources/init.py 100% 🟢
funderfinder/sources/_finder.py 86% 🟢
funderfinder/utils/init.py 100% 🟢
tests/sources/test__finder.py 83% 🟢
tests/sources/test_github_sponsors.py 92% 🟢
TOTAL 92% 🟢

Modified Files

File Coverage Status
funderfinder/sources/github_sponsors.py 55% 🟢
funderfinder/sources/numfocus.py 69% 🟢
tests/sources/test_numfocus.py 100% 🟢
TOTAL 75% 🟢

updated for commit: 3e9773d by action🐍

jmelot commented 1 year ago

@jspeed-meyers Thanks for your earlier comments, I think I addressed them. For the sake of completeness, I propagated these changes to the github sponsors as well (I hope you don't mind!). Can you please take a look and let me know if you are ok with what I did there (sorry the diff is so painful to look at because of the indentation change - basically I added a run method that follows what is done in the other scrapers, and also moved a bit of parsing functionality out into the base class)? If so I think we can merge this!

jspeed-meyers commented 1 year ago

LGTM. Merging!