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

Fixes #2688: Skip non-audio media from MusicBrainz #2775

Merged
merged 1 commit into from
Dec 31, 2017

Conversation

nguillaumin
Copy link
Contributor

Some releases have non-audio media, such as CD+DVD or CD+DVD-Video. Skip
these media when fetching album info as they will never match audio
tracks and will always report missing tracks.

I took the naive approach of cherry-picking a list of media suspected to
not contain audio from the MusicBrainz formats list:

https://musicbrainz.org/doc/Release/Format

@nguillaumin
Copy link
Contributor Author

This is the first time I do something in Python, so let me know if I did anything wrong, thanks!

@nguillaumin nguillaumin force-pushed the skip-non-audio-media branch 3 times, most recently from 8811be6 to 4c847fe Compare December 30, 2017 21:19
Some releases have non-audio media, such as CD+DVD or CD+DVD-Video. Skip
these media when fetching album info as they will never match audio
tracks and will always report missing tracks.

I took the naive approach of cherry-picking a list of media suspected to
not contain audio from the MusicBrainz formats list:

https://musicbrainz.org/doc/Release/Format
@sampsyo
Copy link
Member

sampsyo commented Dec 31, 2017

This looks exactly right! Thanks for including the tests, which look very reasonable, and even the changelog entry. Woohoo! ✨

Congratulations on a successful first foray into Python. 🐍 🎉

@sampsyo sampsyo merged commit a5fc80e into beetbox:master Dec 31, 2017
@nguillaumin
Copy link
Contributor Author

Great thanks!

@nguillaumin nguillaumin deleted the skip-non-audio-media branch December 31, 2017 01:20
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.

2 participants