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] Update documentation (A005) #16097

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

Conversation

ntBre
Copy link
Contributor

@ntBre ntBre commented Feb 11, 2025

Follow-up to #15951 to update

  • the options links in A005 to reference lint.flake8-builtins.builtins-strict-checking
  • the description of the rule to explain strict vs non-strict checking
  • the option documentation to point back to the rule

Follow-up to #15951 to update
* the options links in A005 to reference `lint.flake8-builtins.builtins-strict-checking`
* the description of the rule to explain strict vs non-strict checking
* the option documentation to point back to the rule
@ntBre ntBre added the documentation Improvements or additions to documentation label Feb 11, 2025
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.

@ntBre ntBre requested a review from MichaReiser February 11, 2025 14:00
/// `a/logging.py`, `a/b/logging.py`, and so on would all clash with the builtin `logging` module.
/// With the [`lint.flake8-builtins.builtins-strict-checking`] option set to `false`, the module
/// path is considered, so only a top-level `logging.py` or `logging/__init__.py` will trigger the
/// rule and `utils/logging.py`, for example, would not.
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps re-use example from above a/logging.py (or use the utils/logging.py example above)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a good idea, thanks! I went for a consistent use of the logging example:

/// By default, only the last component of the module name is considered, so `logging.py`,
/// `utils/logging.py`, and `utils/logging/__init__.py` would all clash with the builtin `logging`
/// module. With the [`lint.flake8-builtins.builtins-strict-checking`] option set to `false`, the
/// module path is considered, so only a top-level `logging.py` or `logging/__init__.py` will
/// trigger the rule and `utils/logging.py`, for example, would not.

What do you think?

Comment on lines +26 to +31
/// By default, only the last component of the module name is considered, so `logging.py`,
/// `utils/logging.py`, and `utils/logging/__init__.py` would all clash with the builtin `logging`
/// module. With the [`lint.flake8-builtins.builtins-strict-checking`] option set to `false`, the
/// module path is considered, so only a top-level `logging.py` or `logging/__init__.py` will
/// trigger the rule and `utils/logging.py`, for example, would not.
///
Copy link
Member

Choose a reason for hiding this comment

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

We should document here or on the option the difference between preview and non preview. The way I remember it is that the default changes based on preview mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants