-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add package.nls.json checking to find-missing-translations script #8255
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
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.
Reviewing my own code is like grading my own homework - suspiciously perfect yet somehow still needs work.
|
@roomote-agent implement the PR feedback. |
|
Hi @brunobergher! I see the PR feedback and will implement the following improvements:
Working on these changes now! |
- Extract shared JSON parsing utility to reduce code duplication - Add validation for flat JSON structure in package.nls files - Add warning message when package.nls.json is not found - Convert to async/await for parallel file reading for better performance - Improve error handling and consistency across all areas
4960fe6 to
b59f20c
Compare
|
Hi @brunobergher! I have successfully implemented all the PR feedback: ✅ Changes Implemented
📊 Test ResultsThe script is working correctly and detecting the 2 missing translation keys across all 17 language files:
The check-translations CI job is failing as expected due to these missing translations, which is the intended behavior to enforce translation completeness. All other CI checks are passing successfully! 🎉 |
daniel-lxs
left a comment
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.
LGTM!
This PR addresses the issue where the find-missing-translations script was not checking the package.nls.json files for missing translations.
Changes Made
checkPackageNlsTranslations()function to check all package.nls.*.json files against the base package.nls.jsonTesting
The script was tested and successfully detected 2 missing translation keys across all 17 language-specific package.nls files:
commands.preventCompletionWithOpenTodos.descriptionsettings.newTaskRequireTodos.descriptionUsage
This enhancement ensures that all translation files, including the VSCode package.nls files, are properly checked for missing translations.
Important
Add
package.nls.jsonchecking tofind-missing-translations.jsscript, including new area option and error handling.package-nlsarea option tofind-missing-translations.js.checkPackageNlsTranslations()to comparepackage.nls.*.jsonfiles againstpackage.nls.json.bothtoallto includepackage-nlsby default.package.nlsfiles.package.nlsfiles.node scripts/find-missing-translations.js --area=package-nlsto check onlypackage.nlsfiles.node scripts/find-missing-translations.js --area=allto check all areas.node scripts/find-missing-translations.js --area=package-nls --locale=frto check a specific locale.This description was created by
for 496ce25. You can customize this summary. It will automatically update as commits are pushed.