Run runlint.bat on scons and .pyw files#19922
Run runlint.bat on scons and .pyw files#19922christopherpross wants to merge 6 commits intonvaccess:masterfrom
Conversation
These files were not being recognized as Python by pre-commit CI on Linux, so they were never auto-formatted.
There was a problem hiding this comment.
Pull request overview
Formats previously-unlinted SCons scripts and a .pyw entrypoint by applying runlint.bat output, and records the formatting commit in .git-blame-ignore-revs to keep git blame useful.
Changes:
- Applied automated formatting to
sconstructand multiple*sconscriptfiles. - Applied automated formatting to
source/_bridge/runtimes/synthDriverHost/main.pyw. - Added the formatting commit to
.git-blame-ignore-revs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| source/_bridge/runtimes/synthDriverHost/main.pyw | Formatting-only changes (blank lines, trailing comma) consistent with lint/format tooling. |
| sconstruct | Formatting-only changes; removed unused imports; reflowed long calls for readability. |
| nvdaHelper/sonic/sconscript | Formatting-only (string quoting). |
| nvdaHelper/javaAccessBridge/sconscript | Formatting-only (wrapped long env.Command call). |
| nvdaHelper/espeak/sconscript | Formatting-only (trailing whitespace cleanup, blank line). |
| nvdaHelper/archBuild_sconscript | Formatting-only (line wrapping, quoting). |
| .git-blame-ignore-revs | Adds the formatting commit hash so blame ignores mass-format changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| lambda target, source, env: ( | ||
| open(buildVersionFn, "w", encoding="utf-8").write( | ||
| "version = {version!r}\n" | ||
| "publisher = {publisher!r}\n" | ||
| "updateVersionType = {updateVersionType!r}\n" |
There was a problem hiding this comment.
Optional: this uses open(...).write(...) without an explicit close. Using a context manager (or Path.write_text) would guarantee the handle is closed before later build steps (e.g., cleanup/Delete on Windows) and is clearer to readers.
|
The comments from Copilot look valid to me, but I think they are out of scope for this PR since these are pre-existing issues, not introduced by the formatting changes. I'll leave them open and let the reviewers decide. |
|
now that the pre-commit PRs have been merged, can we specify them in pre-commit ci config so these are auto linted? |
|
@seanbudd Do you mean adding a I checked and |
|
I think we should reconsider changing the casing to match |
|
@seanbudd Agreed, renaming would be cleaner. I'm not too familiar with SCons yet but would work myself into it. A first search found quite a few references to the current names. Open question: Should we do the renaming in this PR or use a |
|
Does adding shebangs e.g. |
|
@seanbudd Ok, that's genius, I wouldn't have thought of that. Yes, it works. I tested it locally: if we add a shebang to the scons files, |
|
great! please do that and thank @SaschaCowley and @michaelDCurran for the idea 😄 |
SCons files use magic globals (Import, Export, env, Dir, etc.) injected at runtime, causing false positives in pyright. They also don't contain translatable strings, so checkPot doesn't apply.
christopherpross
force-pushed
the
fix-ruff-format-scons-files
branch
from
6517857 to
b3f4710
Compare
|
@seanbudd Done, added shebangs to all scons files and excluded them from checkPot and pyright since neither applies to scons. Thanks @SaschaCowley and @michaelDCurran for the idea, and thanks for taking the time to review this! |
Must be merge commit, not squash merge (to preserve the commit hash in
.git-blame-ignore-revs)Link to issue number:
No issue created. If a separate issue is needed or anything about this PR should be handled differently, please let me know. I oriented myself on how similar PRs were done in the past (e.g. #16803, #16936).
Summary of the issue:
runlint.baton a clean master produces changes in 6 files. As discussed in #19907, these files are not recognized as Python by pre-commit CI on Linux (see also pre-commit/identify#563), so they were never auto-formatted. Same applies toadd-trailing-comma.Description of user facing changes:
None
Description of developer facing changes:
None
Description of development approach:
runlint.baton a clean master checkout. Committed the resulting changes and updated.git-blame-ignore-revs.#!/usr/bin/env python3shebangs to all scons files so theidentifylibrary recognizes them as Python. This makes pre-commit CI cover them going forward (thanks @seanbudd for the suggestion, credit to @SaschaCowley and @michaelDCurran for the idea).add-trailing-commaandruff formaton the now-recognized scons files and updated.git-blame-ignore-revs.checkPotandpyrightpre-commit hooks. SCons injects globals (Import,Export,env, etc.) at runtime that pyright can't resolve, and scons files don't contain translatable strings.This PR was developed with AI assistance (Claude Code) with human review and manual testing.
Testing strategy:
Ran
runlint.bat,add-trailing-comma, andruff formatagain after committing -- no further changes produced. Verified all pre-commit hooks pass locally, including onsource/comInterfaces_sconscript(the only scons file undersource/).Known issues with pull request:
None
Code Review Checklist: