feat: Added automated translation checker workflow #558
Conversation
Signed-off-by: Ujwal Akotkar <24bcs012@iiitdwd.ac.in>
|
🎉 Welcome @uju09!
We appreciate your contribution! 🚀 |
WalkthroughAdds a GitHub Actions workflow that runs on pushes to main/dev when Changes
Sequence DiagramsequenceDiagram
participant Git as Git Push\n(main/dev)
participant GHA as GitHub Actions
participant Repo as Repository
participant Issue as Translation\nMapping Issue
participant PR as Pull Request
Git->>GHA: Push containing `untranslated.txt` change
activate GHA
GHA->>Repo: Checkout repository
GHA->>GHA: Resolve merged PR number from commit
alt PR Found
GHA->>Repo: Read `untranslated.txt`
alt File Exists
GHA->>GHA: Parse as JSON keys or newline-separated codes
else File Missing
GHA->>GHA: Use empty list
end
GHA->>Issue: Fetch issue body (default #488)
GHA->>GHA: Parse language mappings (name, code, maintainers)
GHA->>GHA: Build mappings: code → @handles and Language (code) → @handles
rect rgb(220,235,255)
note right of GHA: Outputs & comment construction
GHA->>GHA: Set outputs `handles` & `handles_friendly`
GHA->>GHA: Construct PR comment listing untranslated languages and maintainers
end
GHA->>PR: Post comment to resolved PR number
activate PR
PR->>PR: Comment created
deactivate PR
else PR Not Found
GHA->>GHA: Fail workflow (no PR associated)
end
deactivate GHA
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@M4dhav I request you to review and test it. Thank you. |
|
@M4dhav Do I need to make any changes? |
|
Hey @uju09 , This approach is suboptimal, let's approach it in a different way. We do not need a script as the Secondly, instead of checking past commits, you can scrape #488 and check the issue description for the contributors and handle it accordingly |
|
@M4dhav I will change the approach and commit changes soon. |
…ach) Signed-off-by: Ujwal Akotkar <24bcs012@iiitdwd.ac.in>
uju09
force-pushed
the
feature/translation-checker-workflow
branch
from
13f1869 to
f66491a
Compare
|
@M4dhav I changed the approach. Please let me know if any changes are required. Thank you. |
|
The code looks to be fine, could you test the workflow on your local and share a video for the same please |
|
@M4dhav Surely I will share a video of it. |
|
@M4dhav I tested the workflow and it worked fine. I created an issue in my fork repo and instead of PR, I tested it to add a comment on that issue itself and it completely worked fine. 
resonate.workflow.mov |
|
Please confirm that the Workflow only tags the appropriate contributor, for example, if Gujarati strings are in untranslated.txt, only Gujarati Contributor is tagged |
Signed-off-by: Ujwal Akotkar <24bcs012@iiitdwd.ac.in>
|
@M4dhav I have committed the changes. The workflow specifically tags the contributor of that particular language. resonate.workflow.mov
|
There was a problem hiding this comment.
The table regex is unnecessary as the data is only stored as a list. secondly, This workflow will fail when used on any language where the language code is not equal to the first two letters, for example: Marathi (mr) and Spanish (es). We need to fix this. If you want I can either start adding thr language codes next to the Language so the Regex can match and fetch them, or you can create a map with known language codes within the workflow but this is not recommended.
Thirdly please stop tagging actual contributors in your testing issues, please use dummy contributor usernames
|
Please re-request review after changes |
|
@M4dhav I am sorry for tagging the actual contributors and I by mistakely assumed that every language will start by its first two letter. I am also sorry for that, I should have confirmed that with you. I will make necessary commits. |
Signed-off-by: Ujwal Akotkar <24bcs012@iiitdwd.ac.in>
Signed-off-by: Ujwal Akotkar <24bcs012@iiitdwd.ac.in>
|
@M4dhav Any changes needed? |
Signed-off-by: Ujwal Akotkar <24bcs012@iiitdwd.ac.in>
✅ Actions performedReview triggered.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 365 🏁 Script executed: Length of output: 15 🏁 Script executed: Length of output: 6857 🏁 Script executed: Length of output: 17605 @uju09, I've reviewed the workflow and the implementation looks solid! The logic is well-structured and handles edge cases nicely. However, I found a critical issue that needs to be addressed before this can work correctly: 🚨 Critical Issue: Issue #488 Format MismatchThe workflow expects issue #488 to have language codes in parentheses like: But the current format in issue #488 is: Without the language codes in parentheses, the regex won't match any maintainers, and the workflow will output "(no maintainer mapped)" for all languages! You'll need to update issue #488 to include the language codes before this workflow can function properly. ✅ Strengths
💡 Minor Suggestions
📋 Verification StepsBefore merging, please:
The workflow logic itself is excellent and well-implemented! Once issue #488 is updated with the proper format, this should work perfectly. Great job on the iteration based on feedback! 🎉 🏁 Script executed: Length of output: 275 The script output confirms the format mismatch. As you can see, the current issue #488 uses checkboxes Action Item: Please update issue #488 to use the format shown in the "Suggested format" section above, ensuring each language has its 2-letter code in parentheses. Once that's updated, your workflow will correctly match and tag the appropriate maintainers! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/translation-check.yml (2)
61-74: Hardcoded issue number prevents flexibility across repos/forks.The mapping issue number is hardcoded to 488 (line 65). While this works for the primary repo, forks or other repositories cannot easily customize it without editing the workflow file.
Make the issue number configurable via a repository variable or workflow input:
env: - ISSUE_NUMBER: "488" # Replace with your mapping issue number + ISSUE_NUMBER: ${{ vars.TRANSLATION_MAPPING_ISSUE || '488' }}Then document that repos can set
TRANSLATION_MAPPING_ISSUErepository variable to override.
105-115: Array handling suggestion is reasonable but addresses an edge case that Flutter's actual output format does not produce.Flutter's
gen-l10nproduces untranslated.txt as a JSON object with locale codes as keys and arrays of message IDs as values (e.g.,{ "es": ["msg_1"], "tr": ["msg_2"] }). The current code usingObject.keys()correctly extracts the locale codes["es", "tr"]for this standard format.The array edge case you flagged is technically valid: if untranslated.txt were a JSON array like
["en", "de"], Object.keys() would return indices["0", "1"]instead of the language codes. However, this scenario requires malformed input that violates Flutter's specification. The code already handles truly malformed input gracefully by falling back to newline-separated parsing.The suggested fix to detect and handle arrays is reasonable defensive programming for robustness, though not strictly necessary given Flutter's actual output format. If you'd like to add it for extra safety against manual misconfiguration, the suggested approach is sound.
Signed-off-by: Ujwal Akotkar <24bcs012@iiitdwd.ac.in>
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/translation-check.yml (1)
172-179: Validate PR_NUMBER to fail loudly instead of silently accepting invalid input.Although PR resolution now throws an error (step 1), line 172 still lacks explicit validation. If
PR_NUMBERis somehow empty or undefined,Number(undefined)returnsNaN, which would fail when creating the comment but without a clear error message. Add an explicit check to ensure the PR number is valid and fail immediately if it is not.Apply this diff to validate
PR_NUMBER:const issueNumber = Number(process.env.PR_NUMBER); + if (!issueNumber || isNaN(issueNumber)) { + throw new Error('PR_NUMBER not set or invalid'); + } const body = process.env.COMMENT_BODY || 'No comment generated.'; await github.rest.issues.createComment({
🧹 Nitpick comments (1)
.github/workflows/translation-check.yml (1)
64-64: Document the hardcoded issue number.Line 64 hardcodes issue number
488with an inline comment to replace it. For production clarity, consider adding a comment explaining that this issue must follow the expected format (e.g., "Language (code) by @maintainer") so that the regex on line 91 can parse maintainer mappings correctly. Also document this assumption in a README or workflow comments so future maintainers understand the dependency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/translation-check.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Software Tests
🔇 Additional comments (3)
.github/workflows/translation-check.yml (3)
24-38: ✅ Fixed: PR resolution now throws error to halt execution.The critical issue from prior review has been corrected—line 36 now uses
throw new Error()instead ofcore.setFailed(), ensuring the job terminates immediately if no PR is associated with the commit.
149-149: ✅ Fixed: Grammar error corrected.Line 149 now correctly reads "Please translate these keys:" instead of the previous "Please translate this keys:".
116-123: Current logic is correct—the review comment is mistaken.The ternary condition
testing && handles.length ? [dummy] : handlesproperly distinguishes between scenarios:
- Testing mode with mapped handles: Notifies dummy (safe testing)
- Testing mode without mapped handles: Uses empty array, outputs "no maintainer mapped" (no value in testing unmapped codes)
- Production with handles: Notifies real maintainers
- Production without handles: Outputs "no maintainer mapped"
The
handles.lengthcheck ensures that unmapped codes are handled consistently in both testing and production—when no maintainers exist, there's no point notifying dummy. The suggested change would create inconsistent behavior by forcing dummy notifications for unmapped codes only in testing mode, which defeats the purpose of testing reproducible scenarios.Likely an incorrect or invalid review comment.
Signed-off-by: Ujwal Akotkar <24bcs012@iiitdwd.ac.in>
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/translation-check.yml (1)
64-64: Document the maintainer-mapping issue dependency.The hardcoded
ISSUE_NUMBER: "488"is paired with a helpful inline comment, but the workflow's correctness depends entirely on that issue following the expected format:- Language (code) by @user1 @user2. Per the PR context, issue #488 must be updated to include 2-letter language codes in parentheses for the regex to match and populate maintainers; otherwise all entries will show "(no maintainer mapped)".Consider adding a comment in the workflow or README clarifying this format requirement and linking to the issue to help future maintainers understand the dependency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/translation-check.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Software Tests
🔇 Additional comments (6)
.github/workflows/translation-check.yml (6)
35-38: ✓ Prior critical issue fixed: Proper error handling now halts job execution.The script now correctly uses
throw new Error()instead ofcore.setFailed(), ensuring the job halts immediately if no PR is associated with the commit. This prevents downstream steps from executing with undefinedPR_NUMBER.
149-149: ✓ Prior minor issue fixed: Grammar corrected.The message now correctly reads "Please translate these keys:" (plural form).
172-175: ✓ Prior major issue fixed: Explicit validation prevents posting to wrong PR.The PR number is now properly validated. If
PR_NUMBERis missing or invalid, the script throws an error instead of silently defaulting to PR #1, eliminating the risk of posting comments to the wrong PR.
91-102: Regex and parsing logic are sound.The regex pattern correctly captures multi-format entries (
[a-z]{2,3}language codes, space-separated handles), and the fallback parsing (lines 104–114) gracefully handles both JSON and newline-separated code lists. The mapping tables are built correctly, and unmapped languages output "(no maintainer mapped)" instead of failing silently.
116-131: Testing mode implementation is correct.When
TESTINGis true and maintainers exist for a language, the workflow correctly routes the notification to the dummy user (@translation-bot) instead of tagging real maintainers. Languages with no mapped maintainers still show "(no maintainer mapped)" in both testing and normal modes, which is appropriate for visibility.
40-58: File handling is robust.The step gracefully handles both present and missing
untranslated.txtfiles, properly escaping multi-line output using heredocs. If the file is absent, an empty list is output without error, allowing the workflow to complete and post a notice that no untranslated entries were found.
|
@M4dhav All the necessary changes discussed by us and the suggestion from codeRabbit has been committed. Now I request you to review and merge the PR. |
|
Great work on the PR, thank you for the contribution |
|
✅ PR Closed - Thank You, @uju09!
We appreciate your effort and look forward to more contributions from you! 🤝 |
2 participants
Description
This PR introduces an automated GitHub Actions workflow that detects missing translations after PRs are merged to the
mainordevbranches and notifies the relevant maintainers.What it does:
untranslated.txtgenerated during the translation processKey Features:
untranslated.txtFixes #552
Type of change
How Has This Been Tested?
Local Testing:
actto simulate GitHub Actionsuntranslated.txtis read correctlyWorkflow Testing Plan:
Note: Full end-to-end testing occurs only after PRs are merged.
Post-merge testing plan:
untranslated.txtwith missing keys for one or more languagesAlternative: Can test in a fork with simulated PR merges before using in production.
Test Configuration:
.github/workflows/translation-notifier.ymluntranslated.txtChecklist:
Additional Notes
Maintainer Checklist
github-actionsSummary by CodeRabbit