-
Notifications
You must be signed in to change notification settings - Fork 1
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
Remove bare get_timeinfo, repoint tests to GenericTimeParser #315
Remove bare get_timeinfo, repoint tests to GenericTimeParser #315
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #315 +/- ##
==========================================
+ Coverage 98.14% 98.40% +0.25%
==========================================
Files 11 11
Lines 1024 940 -84
==========================================
- Hits 1005 925 -80
+ Misses 19 15 -4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Have you got any idea why codecov is so upset about the changes? It's flagged lines don't really make any sense to me, but I wonder whether we might want to take a closer look just to be sure...
I'm assuming that, because I've excised a chunk of 100% covered code, any uncovered code is now a larger proportion of the code base, hence the percentage of uncovered code has gone up, hence CodeCov is angry... but I would have thought it was smart enough to be able to realize that all that's happened is some line removal. |
That being said, you're right, the highlighting is confusing. For example, L117-135 of |
OK, fixed the issue. I'd missed the module-level private helper functions that now weren't being used, so weren't being tested, so Codecov was flagging that correctly as a test coverage lowering. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
I came across this extension the other day - might help with that sort of unused function stuff? It basically adds a usage count to function definitions, makes it super easy to see unused functions when traipsing through files. |
Closes #314 .
The simple PR removes
source.utils.get_timeinfo
, which has been superseded byGenericTimeParser
. The related tests have been switched to use thatParser
.