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

465 discogs: Fetch a few more metadata fields #3322

Merged
merged 11 commits into from Jul 1, 2019

Conversation

@thedevilisinthedetails
Copy link

commented Jun 30, 2019

No description provided.

Peter added some commits Jun 16, 2019

Peter
Peter
Peter
Peter
Peter
@sampsyo

This comment has been minimized.

Copy link
Member

commented Jun 30, 2019

Looks good at a high level so far! Can you please leave a comment with some background when you're ready for a review?

@thedevilisinthedetails

This comment has been minimized.

Copy link
Author

commented Jun 30, 2019

Sure. So I'm grabbing the additional fields (release date, genre and release id) mentioned in the issue and storing them in the db. For the release I just extracted it from the URI. Genre behaves the same as style (for consistency) and just grabbing the release date.

@sampsyo
Copy link
Member

left a comment

OK, cool! It looks like the three fields are genre, discogs_release_id, and released_date.

  • genre: Seems harmless enough! We already support this field.
  • discogs_release_id: This will be nice to have. Let's think carefully about consistent naming, however. The current MB equivalent is called mb_albumid. Would it make sense to use the same pattern, i.e., discogs_albumid? I don't feel strongly about any of this, but it's worth weighing. Also, I'm not certain this needs to be a built-in fixed field—maybe it's best left out of library.py if it's never going to have a metadata tag?
  • released_date: We already have release date information in the year, month, and day fields. Is this different information?
beetsplug/discogs.py Outdated Show resolved Hide resolved
beetsplug/discogs.py Show resolved Hide resolved

Peter added some commits Jul 1, 2019

Peter
Peter
Peter
@thedevilisinthedetails

This comment has been minimized.

Copy link
Author

commented Jul 1, 2019

addressed your comments @sampsyo

@sampsyo

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

Awesome; thank you!! Merged with a changelog entry.

@sampsyo sampsyo merged commit 1b41249 into beetbox:master Jul 1, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

sampsyo added a commit that referenced this pull request Jul 1, 2019

Merge pull request #3322 from thedevilisinthedetails/master
465 discogs: Fetch a few more metadata fields

sampsyo added a commit that referenced this pull request Jul 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.