Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Rewrite the
fish
plugin from scratch #5359base: master
Are you sure you want to change the base?
Rewrite the
fish
plugin from scratch #5359Changes from all commits
55ef791
a37643f
f5a81af
baa7ae7
e724946
5f986c5
0f90d5a
4be6e47
1c41aff
ef18590
3c01728
4270739
eb5439c
faec46d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check failure on line 61 in beets/test/fixtures.py
GitHub Actions / Check types with mypy
Check failure on line 61 in beets/test/fixtures.py
GitHub Actions / Check types with mypy
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.
mypy
is failing here becausedb_loc
type becomesbytes | str
whilePath | None
is expected. You can deal with this by moving this logic directly to theLibrary
call: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.
Note that
bytestring_path
cast may not be required since the default value forpath
parameter seems to be a string: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.
self.libdir
starts as aPath
and later gets cast tobytes
. As aPath
, it is only used within this method, so you may want to assign a local variable for it, and setself.libdir
withbytes
later:Note the usage of
bytestring_path
: remember the weird magic/bytes/Windows logic? 😅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.
TestCase
has a methodaddCleanup
which accepts the finalizer function. It can be called in the very beginning, so it should slightly simplify things here. SeeInterestingly, using
addCleanup
is recommended over teardown methods, since this makes sure that the cleanup is always performed. On the other hand teardown may not get called if something fails in thesetUp
method, I thinkThere 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.
I think this should remove the need for the
contexts
list.Check failure on line 493 in beets/test/helper.py
GitHub Actions / Check types with mypy
Check failure on line 494 in beets/test/helper.py
GitHub Actions / Check types with mypy
Check failure on line 498 in beets/test/helper.py
GitHub Actions / Check types with mypy
Check failure on line 499 in beets/test/helper.py
GitHub Actions / Check types with mypy
Check failure on line 503 in beets/test/helper.py
GitHub Actions / Check types with mypy
Check failure on line 504 in beets/test/helper.py
GitHub Actions / Check types with mypy
Check failure on line 508 in beets/test/helper.py
GitHub Actions / Check types with mypy
Check failure on line 509 in beets/test/helper.py
GitHub Actions / Check types with mypy