-
Notifications
You must be signed in to change notification settings - Fork 8
Adding linting checks with mypy. #29
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
Conversation
Thanks a ton for your contribution to the library! I'll definitely check out the changes you suggested soon. |
b8bead5
to
e935d5a
Compare
Could the Tests/run action be re-run? |
Just spotted a problem with the tests not passing. We need to fix it. Feel free to handle it yourself, or I'll push some fixes later that should solve the issue. |
Integrating with pytest. Updating types. updating siginature of order_tracks_by_file_id to raise ValueError if KeyError is raised to ensure the contract of track.file_id when a None value is assigned is preserved. updating tests.yml to specify ubuntu version as the ubuntu-latest [label](actions/runner-images#10807) changing affects which version of the mkvtoolnix [repo](https://mkvtoolnix.download/downloads.html#ubuntu) to download from.
39b9505
to
e37fa37
Compare
Ah, the ubuntu-latest runner tag went back from v24 to v22.
Update README.md by lkfortuna · Pull Request #10807 · actions/runner-images
<actions/runner-images#10807>
I updated the tests.yml to instead of using ubuntu-latest to use
ubuntu-22.04 given that it has to be a stable version relative to which repo
<https://mkvtoolnix.download/downloads.html#ubuntu> mkvtoolnix is being
installed.
I pushed a change, not sure if you need to re-enable the action in the PR.
…On Mon, Oct 28, 2024 at 7:37 AM GitBib ***@***.***> wrote:
Just spotted a problem with the tests not passing. We need to fix it. Feel
free to handle it yourself, or I'll push some fixes later that should solve
the issue.
—
Reply to this email directly, view it on GitHub
<#29 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADN5LL3U57XONQD2BKVC33Z5YOWDAVCNFSM6AAAAABQBR37EOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBRGM2DANJRGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
The tests should pass now. The issue is that ubuntu-latest is a moving tag for which runner it is using. When I first started on this, the ubuntu-latest was pointing to ubuntu 24. It appears they have rolled it back per PR. I therefore updated my PR to specify the ubuntu version to be 22.04 as it is in line with the mkvtoolnix version used (jammy) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #29 +/- ##
==========================================
- Coverage 65.92% 65.91% -0.01%
==========================================
Files 10 10
Lines 895 933 +38
==========================================
+ Hits 590 615 +25
- Misses 305 318 +13 ☔ View full report in Codecov by Sentry. |
Ubuntu version detection is now automatic, and mkvtoolnix gets installed for the right Ubuntu version on its own. |
Added the *py.typed` marker file per PEP 0561. |
Integrating with pytest.
Updating types.