use Ruff for linting #16751
Merged
use Ruff for linting #16751
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seanbudd
commented
Jun 27, 2024
seanbudd
commented
Jun 27, 2024
See test results for failed build of commit 7db2452973 |
WalkthroughThe project has transitioned from the Flake8 linter to the faster and more comprehensive Ruff linter. This change streamlines the linting process, simplifying configuration files and instructions while improving speed and coverage. Associated documentation and scripts have been updated accordingly. Changes
Sequence Diagram(s)The changes primarily involve configuration and script updates, thus no sequence diagrams are necessary. Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (7)
projectDocs/dev/codingStandards.md (3)
Line range hint
18-18: Consider removing the repeated word.The phrase "Indentation must be done with tabs (one per level), not spaces." has a repeated word
Indentationwhich could be a typo.- Indentation Indentation must be done with tabs (one per level), not spaces. + Indentation must be done with tabs (one per level), not spaces.
Line range hint
101-101: Add missing article "the".In the sentence "...consider how the code can be moved in future while retaining backwards compatibility.", "the" is missing before "future."
- ...consider how the code can be moved in future while retaining backwards compatibility. + ...consider how the code can be moved in the future while retaining backwards compatibility.
Line range hint
107-107: Add missing comma.A comma might be missing after "import time" in the sentence "...code which executes at import time."
- ...code which executes at import time. + ...code which executes at import time,projectDocs/dev/contributing.md (4)
Line range hint
22-22: Consider rephrasing for standard usage.The phrase "in regards to" is typically considered nonstandard. Consider using "regarding" or "concerning."
- ...considered in regards to implementation, design, and user experience... + ...considered regarding implementation, design, and user experience...
Line range hint
59-59: Strengthen wording.Consider using a stronger verb than "speed up" in the context of accelerating the review process.
- Being proactive will really help to speed up the process of code review. + Being proactive will greatly accelerate the process of code review.
Line range hint
60-60: Add missing comma.A comma is missing after "approved" in the sentence "When the PR is approved it will be merged..."
- When the PR is approved it will be merged... + When the PR is approved, it will be merged...
Line range hint
107-107: Correct verb usage.The word "checkout" is used incorrectly as a verb. It should be "check out."
- Please checkout our [technical design overview](../design/technicalDesignOverview.md). + Please check out our [technical design overview](../design/technicalDesignOverview.md).
See test results for failed build of commit 1632f19c43 |
See test results for failed build of commit 33d993cf79 |
|
I'm so intensely exited with this pr, congratulations! |
LeonarddeR
reviewed
Jun 27, 2024
gerald-hartig
approved these changes
Jun 28, 2024
See test results for failed build of commit 57496359fa |
This was referenced Jun 28, 2024
seanbudd
added a commit
that referenced
this pull request
Jun 28, 2024
Lint the repository with Ruff This is a follow up for #16751 Must be merge commit not squash merge Using #16751, the following commands were applied: ruff check --fix to apply automatic fixes ruff check --add-noqa to add no-qa comments to all lint failures, so that issues that can't be automatically fixed don't trigger future failures
seanbudd
added a commit
that referenced
this pull request
Jul 3, 2024
) Fix up for #16751 Summary of the issue: Ruff recently updated, so we might as well use the latest version. a warning is omitted for the section we added logger-objects to in ruff's config. it was unclear in docs how to avoid pre-commit hooks from triggering. more pre-commit hooks could be added ruff whitespace formating is done through ruff format currently we are only linting with ruff check. We should also run a ruff format on the whole NVDA. ruff was missing scons files, which should also be linted in a mass lint. Summary of other changes: Added pre commit hooks for unit tests and translation comment checks. Added generic pre-commit hooks for python static syntax checking, basic whitespace rules, and file write safety checks change the output of rununittests.bat to have simpler buffer output, only show failures. Ensure the verbose log is logged in AppVeyor. add ruff format to lint checks add scons files to lint checks update coderabbit prompt for change log entry items
5 tasks
seanbudd
added a commit
that referenced
this pull request
Jul 4, 2024
run ruff format to fix whitespace changes Follow up to #16767 Closes #12261 Summary of the issue: When migrating to ruff in #16751, we failed to lint scons files, and also run ruff format on the repository. Ruff format fixes whitespaces issues in python files. Description of user facing changes None Description of development approach Perform lint fixes on scons files and run ruff format.
Merged
seanbudd
added a commit
that referenced
this pull request
Jul 11, 2024
Summary of the issue: The documentation for linting was fragmented. It was not clear why a lint.md file was created as part of #16751 and related changes.. It was probably a result from migrating the Flake8 wiki to the document, and then whittling it down to the point where it became useless. information on how to actually run the linter was missing from it Description of development approach removes lint.md change references to the automated testing doc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fixes #14817
Summary of the issue:
Our linting system has the following issues:
Description of user facing changes
runlint.batDescription of development approach
To lint the whole code base:
ruff check --fixto apply automatic fixesruff check --add-noqato add no-qa comments to all lint failures, so that issues that can't be automatically fixed don't trigger future failures.git-ignore-blame-revsSee #16752 for an example
Testing strategy:
Known issues with pull request:
Code Review Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores