Skip to content

Conversation

@cbachhuber
Copy link
Contributor

Summary

Currently, the proposed fix for https://docs.astral.sh/ruff/rules/print/ violates https://docs.astral.sh/ruff/rules/root-logger-call/. Thus, let's change the proposal to make LOG015 happy as well.

Test Plan

Test manually in a project that has both T201 and LOG015 enabled and run them over the previous and proposed code. Is there continuous testing of the code snippets from the docs?

Currently, the proposed fix violates https://docs.astral.sh/ruff/rules/root-logger-call/. Thus, let's change to proposal to make it happy as well.
Comment on lines 39 to +40
/// logging.basicConfig(level=logging.INFO)
/// logger = logging.getLogger(__name__)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will prepend the file and log level to any log message. If that's not ok, we can

Suggested change
/// logging.basicConfig(level=logging.INFO)
/// logger = logging.getLogger(__name__)
/// logging.basicConfig(level=logging.INFO, format="%(message)s")
/// logger = logging.getLogger(__name__)

@cbachhuber cbachhuber marked this pull request as ready for review December 18, 2025 20:11
@ntBre ntBre added the documentation Improvements or additions to documentation label Dec 18, 2025
@ntBre ntBre requested a review from amyreese December 18, 2025 20:13
@astral-sh-bot
Copy link

astral-sh-bot bot commented Dec 18, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Member

@amyreese amyreese left a comment

Choose a reason for hiding this comment

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

I know why it's there, but IMO the suggested fix shouldn't include logging.basicConfig() at the global module level due to the race condition around which call "wins" based on which module is imported first.

Otherwise, I whole-heartedly agree with this change.

@amyreese amyreese merged commit b342f60 into astral-sh:main Dec 19, 2025
38 checks passed
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