-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix musicbrainz plugin documentation #6024
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
Fix musicbrainz plugin documentation #6024
Conversation
clarifies that musicbrainz must be on plugin list to consider two sources of metadata
help clarify need to explicitly define musicbrainz tagger on a modified plugin list.
|
🧙 Sourcery has finished reviewing your pull request! Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `docs/guides/tagger.rst:292` </location>
<code_context>
.. _the musicbrainz database: https://musicbrainz.org/
+If you recieve a "No matching release found" message from the Auto-Tagger for an album you know is present in MusicBrainz, check that musicbrainz is in the plugin list. Until version `v2.4.0`_ the default metadate source for the Auto-Tagger, the :doc:`musicbrainz plugin </plugins/musicbrainz>`, had to be manually disabled. At present, if the plugin list is changed, musicbrainz needs to be added to the plugin list in order to continue contributing results to Auto-Tagger.
+
If you think beets is ignoring an album that's listed in MusicBrainz, please
</code_context>
<issue_to_address>
**issue (typo):** Correct 'recieve' to 'receive' and 'metadate' to 'metadata'.
Please update the spelling of 'recieve' to 'receive' and 'metadate' to 'metadata' in the documentation.
```suggestion
If you receive a "No matching release found" message from the Auto-Tagger for an album you know is present in MusicBrainz, check that musicbrainz is in the plugin list. Until version `v2.4.0`_ the default metadata source for the Auto-Tagger, the :doc:`musicbrainz plugin </plugins/musicbrainz>`, had to be manually disabled. At present, if the plugin list is changed, musicbrainz needs to be added to the plugin list in order to continue contributing results to Auto-Tagger.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
Heya! Thanks for the PR. I agree we might need a bit more documentation on that.
By default the config is created with |
|
Thanks for pointing that out Sebastian - I'll see if I can get a warning set up on that check, and lint the docs on this PR. |
|
Glad to have you onboard! You should be able run the doc formatter with |
|
Few more commits than I'd have liked, but hey at least I'm familiar with beet's linting flow! I'll look into that lack of warning for no enabled metadata tagger, but solving that is probably better left to a different bug / PR. |
I feel you takes a bit to get used to but makes maintenance alot easier for us in the long run 😄 |
|
Thank you for submitting the fix yourself and taking the time to learn our system. |
Add several lines to documentation to clear up possible confusion on musicbrainz plugin being disabled when plugin list is modified. closes beetbox#6020
Description
Fixes #6020
Add several lines to documentation to clear up possible confusion on musicbrainz plugin being disabled when plugin list is modified.
To Do
I'm not too familiar with the general roadmap or ideology behind beets yet, but there might be a later benefit to later looking into making the config.yaml generate with the musicbrainz plugin filled out by default, instead of it silently being enabled or disabled.
Another option could be to throw a warning if the Auto-Tagger doesn't detect a valid database source to query from.
Thanks for making getting my toes into contributing to open source feel easy - and let me know if there are any further changes or fixes I can apply.
docs/to describe it.)docs/changelog.rstto the bottom of one of the lists near the top of the document.)Tests. (Very much encouraged but not strictly required.)