Replace scons checkPot with standalone runcheckpot.bat#19676
Merged
Conversation
38464e8 to
2259cae
Compare
Remove the checkPot target from sconstruct and provide a standalone runcheckpot.bat script that calls scons pot then validates the output with tests/checkPot.py. This follows up on PR nvaccess#19606 review feedback to stop using SCons for checkPot entirely. Also fixes tests/checkPot.py __main__ to call sys.exit() with the error count so exit codes propagate correctly when run standalone.
2259cae to
de53c3d
Compare
seanbudd
approved these changes
Feb 23, 2026
Properly return a 0 or 1 exit code (0=success, 1=failure). Bring back the print statement taht prints number of errors to stdout. Co-authored-by: Sean Budd <seanbudd123@gmail.com>
The checkPot function already prints a detailed summary to stderr, making the bare error count print to stdout redundant.
Contributor
Author
|
@seanbudd Ah, I was considering moving the output outside of the function earlier and didn't properly revert that. Thanks for catching. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the checkPot SCons target and replaces it with a standalone runcheckpot.bat utility that generates nvda.pot via scons pot and then validates translator comments via tests/checkPot.py. It also updates CI, pre-commit, and documentation to reference the new entry point.
Changes:
- Add
runcheckpot.batand switch CI / pre-commit hook to use it instead ofscons checkPot. - Remove the
checkPottarget/action/alias fromsconstruct. - Update
tests/checkPot.pyCLI behavior and update developer/user documentation references.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
runcheckpot.bat |
New wrapper to build the POT via SCons and run tests/checkPot.py validation. |
sconstruct |
Removes the checkPot target wiring now that it’s a standalone script. |
tests/checkPot.py |
Updates __main__ exit behavior for standalone execution. |
.pre-commit-config.yaml |
Updates the checkPot hook to run runcheckpot.bat. |
ci/scripts/tests/translationCheck.ps1 |
Switches translation check CI step to the new batch script. |
projectDocs/testing/automated.md |
Documents the new command for translation string checks. |
projectDocs/dev/contributing.md |
Updates contributor guidance to use runcheckpot.bat. |
projectDocs/dev/buildingNVDA.md |
Updates the threading example commands. |
user_docs/en/changes.md |
Notes the replacement of scons checkPot with runcheckpot.bat. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or 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
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.
Remove the checkPot target from sconstruct and provide a standalone
runcheckpot.bat script that calls scons pot then validates the output
with tests/checkPot.py. This follows up on PR #19606 review feedback
to stop using SCons for checkPot entirely.
Also fixes tests/checkPot.py main to call sys.exit() with the
error count so exit codes propagate correctly when run standalone.
Link to issue number:
N/A
Summary of the issue:
scons checkPot is more a utility and not really part of the build. Having a separate
runcheckpot.batis consistent with other utilities likerununittestsorrunlint. This has been suggested in a comment on PR #19606. Since that PR was already merged, this change is done in this new PR.Description of user facing changes:
None
Description of developer facing changes:
The scons checkPot command has been replaced with runcheckpot.bat. This script generates the pot file via scons pot, then runs tests/checkPot.py directly on the output. Any arguments (e.g. --all-cores, -j 1) are forwarded to the scons pot invocation. Since we need an up-to-date pot file to run checks and generating that file is part of the SCons build, we still have the overhead of calling SCons when running
runcheckpot.bat.Description of development approach:
__main__block to call sys.exit() with the error count. Previously it only printed the count, which meant standalone invocation always exited 0 regardless of errors. This was not a problem when SCons used the returnvalue directly, but is critical now that the script is invoked via bat file.
Testing strategy:
Known issues with pull request:
None
Code Review Checklist: