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

discogs: set track.medium correctly for single medium albums #2228

Merged
merged 3 commits into from
Oct 19, 2016

Conversation

diego-plan9
Copy link
Member

This pull requests mean to carry on the job by @kooimens at the open pull request #1691, adding some tests and an updated changelog. I cherry-picked the commit from kooimens in order to preserve credit.

As hinted on #2222, the problem was that medium_count was correctly increased whenever a new medium was detected on the main loop, but for albums with only one medium that increase did not take place, ending up with a track.medium = 0.

diego-plan9 and others added 3 commits October 17, 2016 19:48
Add three tests for the setting of tracks' medium and medium_total on
the discogs plugin. test_parse_medium_numbers_single_medium is meant
to fail due to beetbox#587.
Fixes beetbox#587. The disc field is only zero when there is only one medium, so I think this will do the trick. I wasn't able to reproduce the real problem within the code. This is just a small workaround.
@sampsyo
Copy link
Member

sampsyo commented Oct 17, 2016

Very nice. With these tests, I'm convinced!

@diego-plan9
Copy link
Member Author

Awesome, I'm glad the tests are persuasive enough! 🍸 Would anything else be required for this fix, or on the contrary would it be ok to merge it (and cleanup the related issue and the previous PR)?

@sampsyo
Copy link
Member

sampsyo commented Oct 19, 2016

Nope! I say go ahead and merge at will.

@diego-plan9
Copy link
Member Author

Excellent! Merging then!

@diego-plan9 diego-plan9 merged commit 6402e05 into beetbox:master Oct 19, 2016
@diego-plan9 diego-plan9 deleted the discogs-disc0 branch October 19, 2016 12:59
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