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.

Sign up for GitHub

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

Jump to bottom

(📚) documentation for B028 is misleading #14289

Open
KotlinIsland opened this issue Nov 12, 2024 · 5 comments · May be fixed by #14338
Open

(📚) documentation for B028 is misleading #14289

KotlinIsland opened this issue Nov 12, 2024 · 5 comments · May be fixed by #14338
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@KotlinIsland
Copy link
Contributor

KotlinIsland commented Nov 12, 2024 •
edited
Loading

The warnings.warn method uses a stacklevel of 1 by default, which limits the rendered stack trace to that of the line on which the warn method is called.

https://docs.astral.sh/ruff/rules/no-explicit-stacklevel/#no-explicit-stacklevel-b028

reading this, it implies that if a higher number is set, then the stack trace will contain multiple entries

It's recommended to use a stacklevel of 2 or higher, give the caller more context about the warning.

- give
+ giving
OR
+ to give

maybe

The `warnings.warn` method uses a stacklevel of 1 by default, which will output a stack frame of the line on which the `warn` method is called. Setting it to a higher number will output a stack frame from higher up the stack.

It's recommended to use a `stacklevel` of 2 or higher, to give the caller a more appropriate context about the warning.
@dylwil3 dylwil3 added documentation Improvements or additions to documentation good first issue Good for newcomers labels Nov 12, 2024
@glthm
Copy link

glthm commented Nov 14, 2024

Has this been included in one of your branches yet @dylwil3?

@dylwil3
Copy link
Collaborator

dylwil3 commented Nov 14, 2024

Nope! Hoping for a new contributor to take it as an excuse to pull down the repo and get comfortable with the PR process 😄

@glthm
Copy link

glthm commented Nov 14, 2024

Yeah I was starting to look at it but the PR process is a bit complex so I won't be able to get into it during this month, so anyone can feel free to do it

@njhearp njhearp linked a pull request Nov 14, 2024 that will close this issue
Open
@njhearp
Copy link

njhearp commented Nov 14, 2024

I thought I would try this to learn the process for a pull request. Sorry about the mess with the commits.

@dylwil3
Copy link
Collaborator

dylwil3 commented Nov 14, 2024

  • @glthm Please don't hesitate to reach out and I can help navigate the PR process! Also would love feedback if the CONTRIBUTING docs could be made more helpful / less intimidating 😄

  • Thanks @njhearp! No need to apologize for the commits - great contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[B028-doc-update] Update documentation for B028 njhearp/ruff
4 participants
@dylwil3 @glthm @KotlinIsland @njhearp