-
Notifications
You must be signed in to change notification settings - Fork 37
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
Enhancement #317 mark-as-read-backfill #333
Conversation
…ntries in a feed, and not only new ones. Added tests.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #333 +/- ##
==========================================
- Coverage 95.16% 91.90% -3.26%
==========================================
Files 95 95
Lines 11611 11656 +45
Branches 2179 1539 -640
==========================================
- Hits 11050 10713 -337
- Misses 478 875 +397
+ Partials 83 68 -15 ☔ View full report in Codecov by Sentry. |
I think something's wrong with /src/reader/_storage/_sqlite_units.py, I ran the type checking with mypy for my commits and the main branch and I get the same three errors. |
Hi, I'm currently away from a computer. I'll be able to take a closer look at this starting with Wednesday. Re: typing failures: This is likely caused by a new version of mypy being released, feel free to ignore it (IIRC plugins are excluded from type checking). |
mypy issue fixed in ccf010c, rebasing on top of latest should fix it here too. |
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.
Thank you for the PR, much appreciated!
I added a few comments, mainly around making the new code a bit simpler.
If you're in a hurry, feel free to ignore the comments marked with "Nit:".
Sorry I took so long, I was on international travel and it took me some time to figure out the changes. Let me know if there's anything wrong. I'm not entirely sure what's going on with the make.bat |
No worries, thank you for looking into it. Will be back soon with another commit fix the coverage and the make.bat issue. |
Sounds good, thanks for everything! |
#317
I did the first way you suggested.