Ruff changes, expand pre-commit hooks to checkPot and unit tests#16767
Ruff changes, expand pre-commit hooks to checkPot and unit tests#16767
Conversation
WalkthroughThe recent update primarily deals with enhancing linters and documentation within the project. Specifically, the Changes
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
projectDocs/dev/lint.md (1)
Line range hint
9-9: Consider adding a comma for improved readability.To enhance the readability of the sentence, consider adding a comma after "results" in the sentence:
- For faster lint results or greater integration with your tools you may want to set up Ruff with your IDE. + For faster lint results, or greater integration with your tools you may want to set up Ruff with your IDE.This change aligns with the static analysis suggestion.
Tools
LanguageTool
[uncategorized] ~26-~26: Possible missing comma found.
Context: ...configTo avoid pre-commit hooks from triggering use the--no-verify` CLI option. Examp...(AI_HYDRA_LEO_MISSING_COMMA)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
86c461b to
034befe
Compare
See test results for failed build of commit 9b142c3627 |
| md %testOutput% | ||
|
|
||
| call "%scriptsDir%\venvCmd.bat" py -m xmlrunner discover -v -s "%unitTestsPath%" -t "%here%" --output-file "%testOutput%\unitTests.xml" %* | ||
| call "%scriptsDir%\venvCmd.bat" py -m xmlrunner discover -b -s "%unitTestsPath%" -t "%here%" --output-file "%testOutput%\unitTests.xml" %* |
There was a problem hiding this comment.
Why the change from -v to -b?
There was a problem hiding this comment.
the test success output is incredibly noisy, to the point where it cannot be parsed reasonably. -b only shows test failures, and makes it easier to parse for both sighted and screen reader devs. the buffer output for screen reader users is F for each test failure, and without supressing punctuation, a "dot" for each test success.
we use -v for appveyor to retain a full log.
See test results for failed build of commit d49215a8c0 |
| "*.py", | ||
| "*.pyw", | ||
| "sconstruct", | ||
| "*sconscript", |
There was a problem hiding this comment.
Ugh, I now notice that this gets really noisy. We should either tell Ruff about scons internals like Return, Import, etc. or revert this, I think.
There was a problem hiding this comment.
This seems fixable with
[tool.ruff.lint.per-file-ignores]
# sconscripts contains many inbuilt functions not recognised by the lint,
# so ignore F821.
"sconstruct" = ["F821"]
"*sconscript" = ["F821"]
Closes #11905 Follow up to #16767 Summary of the issue: scons files have a lot of inbuilt functions that the linter doesn't recognize. See also #11905 Ruff preserves indent/new-line formatting of function arguments, list items, and similar iterables, if a trailing comma is added. Much of NVDA is missing these trailing commas causing the lint to collapse them. Description of user facing changes Description of development approach A lint rule has been added to ignore F821 errors in scons files. A pre-commit hook is added, which adds a trailing comma to python in case it was missed.
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.
Follow up to #16767, #16803 Preparation for #16852 Summary of the issue: Pre-commit touches several files it shouldn't touch, e.g. symbol dictionaries, translated markdown. Description of user facing changes None Description of development approach Update Ruff to 0.5.2 in requirements Add a hook for scons source Update sconstruct to automatically use the max number of cores when passing -j 0, which can be used with pre-commit to have optimal performance Run pre-commit autoupdate
Link to issue number:
Fix up for #16751
Summary of the issue:
logger-objectsto in ruff's config.ruff formatcurrently we are only linting withruff check. We should also run aruff formaton the whole NVDA.Summary of other changes:
@coderabbitai summary