-
-
Notifications
You must be signed in to change notification settings - Fork 738
fix up pre-commit for xliff files #19426
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: beta
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR fixes a pre-commit configuration issue that prevents adding new languages to NVDA from Crowdin. The check-added-large-files hook was blocking commits of large xliff translation files.
- Updates pre-commit config to exclude xliff files from the large file size check
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - id: check-added-large-files | ||
| # POFiles and TTF fonts can't be made smaller | ||
| exclude_types: ["pofile", "ttf"] | ||
| exclude_types: ["pofile", "ttf", ".xliff"] |
Copilot
AI
Jan 9, 2026
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.
The exclude_types syntax is incorrect. The 'identify' library used by pre-commit recognizes file types without the leading dot. According to the pattern used for 'pofile' and 'ttf' (which also don't have dots), this should be 'xliff' instead of '.xliff'. The leading dot will prevent the exclusion from working correctly, and xliff files will still be checked for size, causing the pre-commit hook to fail.
| exclude_types: ["pofile", "ttf", ".xliff"] | |
| exclude_types: ["pofile", "ttf", "xliff"] |
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.
xliff isn't here I'll have to use exclude instead possibly
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.
opened pre-commit/identify#558
Co-authored-by: Copilot <[email protected]>
Link to issue number:
None
Summary of the issue:
Adding new languages to NVDA from Crowdin causes pre-commit to fail, as it commits large xliff files.
Failed job: https://github.com/nvaccess/nvda/actions/runs/20842477976/job/59879416174
Description of user facing changes:
None
Description of developer facing changes:
We can add new languages to NVDA via Crowdin
Description of development approach:
update pre-commit
Testing strategy:
Tested trying to commit a large xliff file locally
Known issues with pull request:
n/a
Code Review Checklist: