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

[flake8-builtins] Remove builtins- prefix from option names #16092

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

InSyncWithFoo
Copy link
Contributor

@InSyncWithFoo InSyncWithFoo commented Feb 11, 2025

Summary

Resolves #15368.

The following options have been renamed:

  • builtins-allowed-modulesallowed-modules
  • builtins-ignorelistignorelist
  • builtins-strict-checkingstrict-checking

To preserve compatibility, the old names are kept as Serde aliases.

Test Plan

cargo nextest run and cargo insta test.

Copy link
Contributor

github-actions bot commented Feb 11, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@InSyncWithFoo InSyncWithFoo force-pushed the builtins-options branch 2 times, most recently from 0eb38e6 to c163482 Compare February 11, 2025 04:44
@MichaReiser MichaReiser added the configuration Related to settings and configuration label Feb 11, 2025
@MichaReiser
Copy link
Member

Thanks. I think the builtins-ignorelist and builtins-allowed-modules in the summary are incorrect.

What we did in the past is to emit a deprecation warning for renamed settings so that we can remove the old names at some point in the future. It probably requires duplicating the fields on the Options structs and adding some custom code in Settings::from_options (I think it's called). Would you mind looking into this?

@MichaReiser MichaReiser added the breaking Breaking API change label Feb 11, 2025
@MichaReiser MichaReiser added this to the v0.10 milestone Feb 11, 2025
@dhruvmanila
Copy link
Member

As an additional reference, I think the infrastructure already exists and lint.extend-ignore is an example of a deprecated setting.

@InSyncWithFoo InSyncWithFoo force-pushed the builtins-options branch 3 times, most recently from 5e78095 to 56895d5 Compare February 11, 2025 18:42
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking API change configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename flake8-builtins options to remove the "builtins" prefix
3 participants