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

Ruff formatter conflicting lint rules #13

Closed
rsyring opened this issue Jul 11, 2024 · 4 comments
Closed

Ruff formatter conflicting lint rules #13

rsyring opened this issue Jul 11, 2024 · 4 comments

Comments

@rsyring
Copy link
Member

rsyring commented Jul 11, 2024

Background: https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules

Our ruff config ignores most of the rules listed on that page that end up being redundant. It leaves the following potentially conflicting rules in place:

because I think implicit string concatenation is too easy to miss when scanning code. I'd prefer concatenation to always be explicit.

There is a discussion going on regarding implicit string concatenation and how to remove the conflict: astral-sh/ruff#8272

Trailing Commas

Explanation and discussion at: astral-sh/ruff#9216 (comment)

I'd prefer to keep COM812 as an active rule and just do a double save when it shows up. I've done this in the past and it's slightly annoying but acceptable IMO. I could see how others would find this very annoying and be resistant. So, do you care either way?

@rsyring
Copy link
Member Author

rsyring commented Jul 11, 2024

I've also not ignored line-too-long because I think that's something worth fixing manually if the linter and formatter can't get it squared away for us.

All of these decisions have been made in the context of these assumptions:

  • Linting is ran first with --fix
  • Formatter is then ran
  • The above steps are taken on save in the devs editor (or ran manually) before a commit is made. I.e. the dev should have the code linted and formatted and that should be part of the local code review, before a commit is made.
  • pre-commit & therefore nox/CI will only flag linting/formatting problems, never make the fix or format.

rsyring added a commit that referenced this issue Jul 11, 2024
@calebsyring
Copy link
Contributor

The double-save doesn't bother me too intensely. I'm frequently reflexively hitting ctrl+s, so I probably won't notice most of the time.

@matthiaswh
Copy link

I have a strong preference for COM812.

@rsyring
Copy link
Member Author

rsyring commented Oct 1, 2024

Sounds like we are good with the current config

@rsyring rsyring closed this as completed Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants