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

Tentative fix for issue 2734: also check for RETAG #2735

Merged
merged 3 commits into from
Nov 10, 2017

Conversation

Vrihub
Copy link
Contributor

@Vrihub Vrihub commented Nov 5, 2017

This is a tentative fix for issue #2734 , based on the suggestion in #2734 (comment)

I've tested it and it fixes the problem for me, but I'm not sure if other changes are required for a complete solution, since my knowledge of the code base is very limited. Please review!

@wisp3rwind
Copy link
Member

Yep, that's exactly the code I meant to suggest.

grep -r RETAG shows that the edit plugin is the only piece of code using this choice and the following comment from ìmporter.py also suggests that the difference between APPLY and RETAG is really only the source of the metadata. Apart from that they both occur for the same kinds of import tasks and the same plugin actions should probably happen.

action = Enum('action',
              ['SKIP', 'ASIS', 'TRACKS', 'APPLY', 'ALBUMS', 'RETAG'])
# The RETAG action represents "don't apply any match, but do record
# new metadata". It's not reachable via the standard command prompt but
# can be used by plugins.

Note that I'm not intimately familiar with the importer, either.

@sampsyo
Copy link
Member

sampsyo commented Nov 9, 2017

This looks great; thank you!

It looks like Travis has pointed out a too-long line: https://travis-ci.org/beetbox/beets/jobs/297512858#L1095

Also, would you mind adding a changelog entry that summarizes the problem that was fixed?

@Vrihub
Copy link
Contributor Author

Vrihub commented Nov 10, 2017

Uhm, no idea why AppVeyor fails here: https://ci.appveyor.com/project/beetbox/beets/build/2228

@sampsyo
Copy link
Member

sampsyo commented Nov 10, 2017

Thank you! ✨ I'm pretty sure that AppVeyor failure is spurious, so I'll merge this now. Woohoo!

@sampsyo sampsyo merged commit 8ca2717 into beetbox:master Nov 10, 2017
@Vrihub Vrihub deleted the 2734-candidates-fetchart branch November 11, 2017 11:37
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.

3 participants