Document link between import-outside-top-level (PLC0415) and lint.flake8-tidy-imports.banned-module-level-imports
#18733
Conversation
…8-tidy-imports_banned-module-level-imports
Avasam
force-pushed
the
document-link-between-import-outside-top-level-PLC0415-and-lint_flake8-tidy-imports_banned-module-level-imports
branch
from
34f0033 to
d194c49
Compare
| /// ## Options | ||
| /// - `lint.flake8-tidy-imports.banned-module-level-imports` | ||
| /// | ||
| /// ## See also | ||
| /// This rule will ignore import statements configured in | ||
| /// [`lint.flake8-tidy-imports.banned-module-level-imports`][banned-module-level-imports] | ||
| /// for the rule [`banned-module-level-imports`][TID253]. |
There was a problem hiding this comment.
Wasn't sure whether to put this directly above ## Example, in ## See also or merge it into ## Options.
| /// ## Options | |
| /// - `lint.flake8-tidy-imports.banned-module-level-imports` | |
| /// | |
| /// ## See also | |
| /// This rule will ignore import statements configured in | |
| /// [`lint.flake8-tidy-imports.banned-module-level-imports`][banned-module-level-imports] | |
| /// for the rule [`banned-module-level-imports`][TID253]. | |
| /// ## Options | |
| /// - This rule will ignore import statements configured in | |
| /// [`lint.flake8-tidy-imports.banned-module-level-imports`][banned-module-level-imports] | |
| /// for the rule [`banned-module-level-imports`][TID253]. |
There was a problem hiding this comment.
Also, I didn't validate, but is this true all the time or only if banned-module-level-imports (TID253) is enabled ?
There was a problem hiding this comment.
I like having a separate section, but I'd probably put ## See also before ## Options.
Oh, I didn't notice this when looking at the rule initially, but this is only true if TID253 is enabled. Good catch! Maybe that means we can just put the whole explanation in See also and not have separate Options since they only apply conditionally. I like your current See also section in that case, possibly with a slight alteration to the last line:
/// if the rule [`banned-module-level-imports`][TID253] is enabled.
| /// block, or some other nested context). This affects both `banned-module-level-imports` | ||
| /// and `import-outside-top-level` rules. |
There was a problem hiding this comment.
I didn't make those links because they appear in the json schema.
There was a problem hiding this comment.
We may need the same caveat about "if TID253 is enabled" here too.
Avasam
changed the title
import-outside-top-level (PLC0415) and lint.flake8-tidy-imports.banned-module-level-imports
|
| /// ## Options | ||
| /// - `lint.flake8-tidy-imports.banned-module-level-imports` | ||
| /// | ||
| /// ## See also | ||
| /// This rule will ignore import statements configured in | ||
| /// [`lint.flake8-tidy-imports.banned-module-level-imports`][banned-module-level-imports] | ||
| /// for the rule [`banned-module-level-imports`][TID253]. |
There was a problem hiding this comment.
I like having a separate section, but I'd probably put ## See also before ## Options.
Oh, I didn't notice this when looking at the rule initially, but this is only true if TID253 is enabled. Good catch! Maybe that means we can just put the whole explanation in See also and not have separate Options since they only apply conditionally. I like your current See also section in that case, possibly with a slight alteration to the last line:
/// if the rule [`banned-module-level-imports`][TID253] is enabled.
| /// block, or some other nested context). This affects both `banned-module-level-imports` | ||
| /// and `import-outside-top-level` rules. |
There was a problem hiding this comment.
We may need the same caveat about "if TID253 is enabled" here too.
Summary
As mentioned in #18728 (comment) CC @ntBre
Test Plan
Run the docs locally as per https://github.com/astral-sh/ruff/blob/a2cd6df429a3e76880a48a5eb8816a86c36927ed/CONTRIBUTING.md#mkdocs