-
Notifications
You must be signed in to change notification settings - Fork 91
enable E501 in ruff #4605
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
base: master
Are you sure you want to change the base?
enable E501 in ruff #4605
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4605 +/- ##
========================================
Coverage 39.37% 39.37%
========================================
Files 878 878
Lines 37664 37664
Branches 6145 5889 -256
========================================
Hits 14829 14829
- Misses 21579 22317 +738
+ Partials 1256 518 -738
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, no comments
|
|
||
| # Ignore rules that were disabled in pylint or would cause too many changes | ||
| ignore = [ | ||
| "E501", # line-too-long (already handled by line-length setting) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think we should do that, because that applies two similar, but not exactly the same checks. we already handle that by the formatting task, so if anything has to be reformatted due to line length, it will cause an error in the formatting ci job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think we should do that, because that applies two similar, but not exactly the same checks.
I don't understand that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean ruff formatting logic is not that strict about the line lengths compared to the linting logic,
e.g. it doesn't complain on that comment line you changed in this PR, otherwise the formatting job would fail
oh i see we don't have a repo-wide formatting check in CI.
but anyways, i don't think we want two CI jobs checking one thing -- it's enough to enforce this in the formatting job (which is commented-out currently)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has ruff format done that change, btw?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has ruff format done that change, btw?
no
but anyways, i don't think we want two CI jobs checking one thing
I don't see any problems with it and I don't want to have long lines with comments like I fixed in this PR (or with long string which AFAICT ruff won't reformat as well)
we have both black and pycodestyle in enterprise and I don't think it caused any issues
also prior to using ruff we had E501-like check in this repo
Greptile Overview
Updated On: 2025-10-15 13:11:15 UTC
Summary
Enabled E501 (line-too-long) enforcement in ruff by removing it from the ignore list, ensuring all Python code adheres to the 119-character line limit.
ruff.tomlignore list to enforce line length checksathena.pyby moving an inline comment to a separate lineConfidence Score: 5/5
Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant Dev as Developer participant Ruff as Ruff Linter participant Code as Codebase Dev->>Ruff: Remove E501 from ignore list Note over Ruff: E501 (line-too-long) now enforced Ruff->>Code: Scan for lines > 119 chars Code-->>Ruff: Found line 113 in athena.py (122 chars) Dev->>Code: Split comment to separate lines Note over Code: Line 113: comment moved<br/>Line 114: code remains Ruff->>Code: Re-scan all files Code-->>Ruff: All lines comply with 119 char limit Ruff-->>Dev: Linting passes ✓