-
-
Notifications
You must be signed in to change notification settings - Fork 848
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
feat: add locale check script and GitHub Action workflow #3817
feat: add locale check script and GitHub Action workflow #3817
Conversation
WalkthroughThis pull request adds an automated locale consistency check to the repository. A new GitHub Actions workflow, triggered on pull requests targeting the Changes
Sequence Diagram(s)sequenceDiagram
participant PR as "Pull Request"
participant GH as "GitHub Actions"
participant Checkout as "actions/checkout"
participant NodeSetup as "actions/setup-node"
participant Npm as "npm install"
participant Script as "checkLocales.js"
participant Locales as "Locale Files"
PR->>GH: Trigger workflow on PR to main/develop
GH->>Checkout: Checkout repository
Checkout-->>GH: Repository checked out
GH->>NodeSetup: Setup Node.js v18 environment
NodeSetup-->>GH: Environment ready
GH->>Npm: Install dependencies
Npm-->>GH: Dependencies installed
GH->>Script: Execute locale check
Script->>Locales: Read primary & target locale JSON files
Locales-->>Script: Return locale data
Script-->>GH: Log results and exit on error/success
Poem
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3817 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 667 667
Branches 113 113
=========================================
Hits 667 667 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 4
🧹 Nitpick comments (5)
.github/workflows/check-locales.yml (3)
15-15
: Update the GitHub Actions checkout action to v4.The checkout action version v3 is outdated. GitHub recommends using the latest version for improved security and features.
- uses: actions/checkout@v3 + uses: actions/checkout@v4🧰 Tools
🪛 actionlint (1.7.4)
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
18-18
: Update the GitHub Actions setup-node action to v4.The setup-node action version v3 is outdated. GitHub recommends using the latest version for improved security and features.
- uses: actions/setup-node@v3 + uses: actions/setup-node@v4🧰 Tools
🪛 actionlint (1.7.4)
18-18: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
22-23
: Consider implementing dependency caching.Adding caching for Node.js dependencies can significantly speed up your workflow execution time.
- name: Install Dependencies + id: cache + uses: actions/cache@v3 + with: + path: '**/node_modules' + key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-node- + + - name: Install Dependencies (if needed) + if: steps.cache.outputs.cache-hit != 'true' run: npm installscripts/checkLocales.js (2)
6-7
: Consider making the directory paths configurable.Hardcoding paths might make the script less flexible if the project structure changes in the future.
-const LOCALES_DIR = path.join(__dirname, '../public/locales'); -const PRIMARY_LOCALE = 'en'; +// Allow overriding via environment variables +const LOCALES_DIR = process.env.LOCALES_DIR || path.join(__dirname, '../public/locales'); +const PRIMARY_LOCALE = process.env.PRIMARY_LOCALE || 'en'; + +console.log(`Checking locales in: ${LOCALES_DIR}`); +console.log(`Primary locale: ${PRIMARY_LOCALE}`);
47-48
: Consider checking for extra keys in locale files.Currently, the script only checks for missing keys, but doesn't verify if the locale files contain unnecessary extra keys that don't exist in the primary locale.
const missingKeys = Object.keys(primaryData).filter(key => !(key in localeData)); + const extraKeys = Object.keys(localeData).filter(key => !(key in primaryData)); + + if (extraKeys.length > 0) { + console.warn(`Extra keys in ${folder}/${file} (not in primary locale): ${JSON.stringify(extraKeys, null, 2)}`); + // Optionally set hasErrors = true if you want to treat extra keys as errors + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/check-locales.yml
(1 hunks)scripts/checkLocales.js
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/check-locales.yml
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
18-18: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
scripts/checkLocales.js (3)
1-2
: Disable ESLint rules with caution.These ESLint disables suppress important code quality checks. Consider addressing the underlying formatting issues instead of disabling the rules entirely. If disabling is necessary, add a comment explaining why.
49-50
: Consider checking for nested objects in locale files.The current implementation only checks for top-level keys. Many locale files use nested structures, which this implementation won't validate correctly.
const missingKeys = Object.keys(primaryData).filter(key => !(key in localeData)); + // Helper function to recursively check nested objects + const checkNestedObjects = (primary, locale, parentKey = '') => { + let nestedMissingKeys = []; + + Object.keys(primary).forEach(key => { + const currentKey = parentKey ? `${parentKey}.${key}` : key; + + if (typeof primary[key] === 'object' && primary[key] !== null && + typeof locale[key] === 'object' && locale[key] !== null) { + // Recursively check nested objects + nestedMissingKeys = [...nestedMissingKeys, + ...checkNestedObjects(primary[key], locale[key], currentKey)]; + } else if (!(key in locale)) { + nestedMissingKeys.push(currentKey); + } + }); + + return nestedMissingKeys; + }; + + // Check for nested missing keys + const nestedMissingKeys = checkNestedObjects(primaryData, localeData); + if (nestedMissingKeys.length > 0) { + console.error(`Missing nested keys in ${folder}/${file}: ${JSON.stringify(nestedMissingKeys, null, 2)}`); + hasErrors = true; + }
1-82
: Consider adding additional validation features.The script currently only checks for missing keys but doesn't validate other important aspects:
- Keys in target locales that don't exist in the primary locale (potentially outdated)
- Value type mismatches between locales (e.g., string vs. object)
- Placeholder consistency (e.g.,
{name}
exists in one locale but not another)Would you like me to provide implementation suggestions for these additional validations?
🧰 Tools
🪛 Biome (1.9.4)
[error] 59-59: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 59-60: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/checkLocales.js
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
scripts/checkLocales.js
[error] 59-59: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 59-60: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
🔇 Additional comments (6)
scripts/checkLocales.js (6)
6-7
: LGTM! The constants are well-defined.The script correctly defines the locales directory path and primary locale constant.
9-16
: The error handling in readJson is now properly implemented.The function correctly reads and parses JSON files, with proper error handling using console.error.
18-30
: Good implementation of folder existence checks.The function correctly checks if both primary and target locale folders exist before proceeding, with appropriate error messages and return values.
32-34
: LGTM! Effective file filtering.The code correctly identifies and filters JSON files in both directories.
35-58
: The error accumulation approach is well-implemented.The code now properly accumulates errors instead of exiting on the first issue, which provides better visibility into all locale inconsistencies at once.
62-80
: Overall flow for checking locales and reporting errors is well-structured.The script effectively:
- Identifies all locale directories
- Checks each non-primary locale against the primary one
- Collects all errors before showing a final status
- Exits with the appropriate code based on validation results
This ensures that all locale issues are reported at once, making it easier for developers to fix them.
}); | ||
}; |
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.
Fix syntax error in the checkLocale function.
There's an extra closing parenthesis and curly brace that will cause a syntax error.
return !hasErrors;
- });
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
}); | |
}; | |
return !hasErrors; | |
}; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 59-59: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 59-60: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
This Issue is under bounty program. Hence PR can't be generated directly. |
Could you please guide me on the correct process to contribute? Should I get prior approval before submitting a PR on bounty program ? |
Description
Implemented a script to check for missing/inconsistent locale variables.
Integrated the script into a GitHub Action workflow for automated checks.
Ensured compatibility with the existing locale structure (separate JSON files inside en/).
Related issue(s)
Resolves #3777
Summary by CodeRabbit