Add deprecation warning for musicbrainz.enabled but use it to load the plugin, centralise deprecations handling
#6127
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes deprecation handling in beets by introducing a new beets/util/deprecation.py module with standardized helper functions, and updates the MusicBrainz plugin loading behavior to always load unless explicitly disabled.
Key Changes:
- Created centralized deprecation system with
deprecate_for_user(),deprecate_for_maintainers(), anddeprecate_imports()functions - Modified MusicBrainz plugin to auto-load when not in plugins list and not disabled, with deprecation warnings
- Migrated all deprecation warnings throughout codebase to use new centralized functions
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| beets/util/deprecation.py | New module providing centralized deprecation helper functions with automatic version calculation |
| beets/util/init.py | Removed old deprecate_imports() function migrated to new deprecation module |
| beetsplug/musicbrainz.py | Updated to use new deprecate_for_user() function for searchlimit option warning |
| beets/ui/init.py | Replaced inline warning with deprecate_for_maintainers() call in decargs function |
| beets/plugins.py | Updated MusicBrainz auto-loading logic and migrated deprecation warnings to new functions |
| beets/mediafile.py | Replaced inline warning with deprecate_for_maintainers() call |
| beets/library/init.py | Updated import and removed version parameter from deprecate_imports() call |
| beets/autotag/init.py | Migrated deprecation warnings to new centralized functions |
| beets/init.py | Updated import and simplified deprecate_imports() call |
| beets/config_default.yaml | Changed default plugins list from [musicbrainz] to [] and added disabled_plugins option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
❌ 83 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Member
|
I like the additional deprecation warnings, but adding a disabled plugin list doesn't feel right to me when a plugin should be considered disabled by merit of not being on the list of plugins to begin with. My thoughts on a solution to the Musicbrainz problem:
|
snejus
changed the title
musicbrainz.enabled but use it to load the plugin, centralise deprecations handling
Member
Author
Agree with you. I have now updated the logic and added a test to show the behaviour (otherwise there's too much to wrap one's head around!) |
snejus
force-pushed
the
always-enable-mb
branch
2 times, most recently
from
2a18445 to
9fdcd0a
Compare
semohr
reviewed
Oct 26, 2025
snejus
force-pushed
the
always-enable-mb
branch
from
9fdcd0a to
60387ea
Compare
snejus
force-pushed
the
always-enable-mb
branch
from
60387ea to
13f8f08
Compare
snejus
force-pushed
the
always-enable-mb
branch
from
13f8f08 to
422f567
Compare
snejus
force-pushed
the
always-enable-mb
branch
from
422f567 to
dd72704
Compare
Member
Author
|
@semohr would be keen to merge it if it looks good. |
semohr
approved these changes
Nov 30, 2025
Contributor
semohr
left a comment
semohr
left a comment
There was a problem hiding this comment.
Sure, seems like a good enough refactor to me!
And update imports that have been raising the deprecation warning.
snejus
force-pushed
the
always-enable-mb
branch
from
a76c976 to
05430f3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #6121
This PR introduces a centralized deprecation system and adjusts
musicbrainzplugin loading to properly handle the deprecatedmusicbrainz.enabledconfiguration option.MusicBrainz
musicbrainz.enabledconfiguration option:true, warns users to explicitly addmusicbrainzto theirpluginsconfiguration and adds it if not already presentfalse, warns users and adds the plugin todisabled_plugins(listreceived by the
--disable-pluginsflag)Deprecations
Created new
beets/util/deprecation.pymodule with standardized deprecation helpers:deprecate_for_user()- logs warnings visible to end usersdeprecate_for_maintainers()- emitsDeprecationWarningfor developersdeprecate_imports()- handles deprecated module imports with automatic version calculation_format_message()- generates consistent deprecation messages that auto-calculate next major versionMigrated all deprecation handling to use the new centralized functions:
warnings.warn()calls throughout codebasedeprecate_imports()signature to remove explicitversionparameterdeprecate_for_user()