Skip to content
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

Support ipfs add --nocopy in ipfs plugin #3135

Merged
merged 3 commits into from
Feb 1, 2019
Merged

Support ipfs add --nocopy in ipfs plugin #3135

merged 3 commits into from
Feb 1, 2019

Conversation

wildthyme
Copy link
Contributor

This is a very minor change, but previously the ipfs plugin was making a separate copy of every file added. I left the default behaviour the same because using this requires setting an additional option in ipfs proper.

docs/changelog.rst Outdated Show resolved Hide resolved
@sampsyo
Copy link
Member

sampsyo commented Jan 30, 2019

Nice! This looks good to me.

If I'm reading it right, the Travis failure here:
https://travis-ci.org/beetbox/beets/jobs/486649015

Is unrelated; it looks like yet another flake8 version bump we need to address.

@jackwilsdon
Copy link
Member

jackwilsdon commented Jan 30, 2019

@sampsyo looks like it's in part due to pep8-naming@0.8.0 (see this issue).

The easiest resolution for now is probably to completely avoid 0.8.0, as a version containing the fix has not been released yet.

@sampsyo
Copy link
Member

sampsyo commented Jan 30, 2019

Good find!! Arg, that's frustrating—but you're correct that this is probably the right fix.

@jackwilsdon
Copy link
Member

jackwilsdon commented Jan 30, 2019

Time to don the detective hat 🕵️

It looks like a new error was added in pycodestyle@2.5.0;

beets/art.py:63:13: E117 over-indented
beetsplug/discogs.py:289:17: E117 over-indented
beetsplug/replaygain.py:938:17: E117 over-indented
beetsplug/filefilter.py:46:17: E117 over-indented
beetsplug/hook.py:94:17: E117 over-indented

These are a bug added in pep8-naming@0.8.0 as I said in my previous comment (🐞 PyCQA/pep8-naming#95);

beets/mediafile.py:1228:35: N803 argument name '_' should be lowercase
beets/mediafile.py:1361:35: N803 argument name '_' should be lowercase
beets/mediafile.py:1384:35: N803 argument name '_' should be lowercase
test/test_thumbnails.py:52:41: N803 argument name '_' should be lowercase
test/test_thumbnails.py:127:53: N803 argument name '_' should be lowercase
test/test_thumbnails.py:185:50: N803 argument name '_' should be lowercase
test/test_thumbnails.py:210:52: N803 argument name '_' should be lowercase
test/test_thumbnails.py:253:47: N803 argument name '_' should be lowercase

This seems to be a bug introduced in pep8-naming@0.8.0 too (🐞 PyCQA/pep8-naming#89);

beetsplug/smartplaylist.py:107:22: N806 variable 'Model' in function should be lowercase

pep8-naming@0.8.0 has changed N81x errors such that they are now picked up in import statements as well;

beetsplug/plexupdate.py:15:2: N814 camelcase 'xml.etree.ElementTree' imported as constant 'ET'
beetsplug/lyrics.py:880:10: N814 camelcase 'xml.etree.ElementTree' imported as constant 'ET'

N815 is a new check in pep8-naming@0.8.0;

beetsplug/metasync/amarok.py:52:6: N815 variable 'queryXML' in class scope should not be mixedCase

And finally, this one seems to be caused by flake8@3.7.0 coming with a newer version of pyflakes;

beetsplug/subsonicupdate.py:81:12: F632 use ==/!= to compare str, bytes, and int literals

It seems like we can't do much about the N803, N806, N814 or N815 errors until an update is released (and a decision is made here regarding the N814 error), except avoiding pep8-naming@>=0.8.0 entirely.

@sampsyo
Copy link
Member

sampsyo commented Jan 31, 2019

Wow, that's quite a few independent issues. I suppose we should avoid this version, but maybe it would also make sense to #noqa those "ET" imports—those seem like a perfectly acceptable violation of a general policy.

@jackwilsdon
Copy link
Member

I'll open a PR to sort out as much as I can 👍we can discuss it more there

@sampsyo
Copy link
Member

sampsyo commented Feb 1, 2019

Looks perfect; thanks again! ✨

@sampsyo sampsyo merged commit c7c90a5 into beetbox:master Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants