feat(i18n): add i18n-localization skill with checker script#660
feat(i18n): add i18n-localization skill with checker script#660bijinfeng wants to merge 2 commits intoiflytek:mainfrom
Conversation
Add new skill for internationalization and localization best practices. Includes comprehensive documentation, best practices guide, and a Python script to detect hardcoded strings and verify locale completeness.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the agent's capabilities by integrating a new skill focused on internationalization and localization. It provides a structured approach to ensuring applications are ready for global audiences, offering detailed guidelines and an automated tool to enforce best practices and identify common i18n issues early in the development cycle. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new skill for i18n and localization, including comprehensive documentation and a Python script for detecting hardcoded strings. The documentation is well-structured and provides good best practices. The checker script is a great addition for maintaining i18n hygiene. I've provided some feedback on the Python script to improve its robustness, error handling, and compatibility, as well as a minor formatting suggestion for the documentation.
| | Ignoring plurals | "1 items" | Use ICU plural | | ||
| | Fixed-width UI | German overflow | Flexible layouts | | ||
| | Text in images | Can't translate | Use CSS/SVG | | ||
| | Gendered pronouns | Not all languages | Use neutral or select | No newline at end of file |
There was a problem hiding this comment.
This file is missing a final newline character. It's a standard convention to end text files with a newline to prevent issues with file concatenation and some command-line tools.
| | Gendered pronouns | Not all languages | Use neutral or select | | |
| | Gendered pronouns | Not all languages | Use neutral or select | | |
| patterns = [ | ||
| "**/locales/**/*.json", | ||
| "**/translations/**/*.json", | ||
| "**/lang/**/*.json", | ||
| "**/i18n/**/*.json", | ||
| "**/messages/*.json", | ||
| "**/*.po", # gettext | ||
| ] |
There was a problem hiding this comment.
The find_locale_files function includes a pattern to find .po files, but the rest of the script, particularly check_locale_completeness, only processes .json files. This can be misleading. To avoid confusion, it's better to either add support for .po files or remove the pattern that finds them. Removing it is the simpler fix if .po file support is not planned.
patterns = [
"**/locales/**/*.json",
"**/translations/**/*.json",
"**/lang/**/*.json",
"**/i18n/**/*.json",
"**/messages/*.json",
]| def is_probably_locale_code(s: str) -> bool: | ||
| return bool(re.match(r'^[a-z]{2,3}(-[A-Z]{2})?$', s)) | ||
|
|
||
| def detect_language_and_namespace(file_path: Path) -> tuple[str, str] | tuple[None, None]: |
There was a problem hiding this comment.
The type hint tuple[str, str] | tuple[None, None] uses the | operator for union types, which was introduced in Python 3.10. For better compatibility with older Python 3 versions (e.g., 3.9), it's recommended to use Union from the typing module. This also applies to other type hints in this file, like int | None on line 257, which could be Optional[int]. You would need to add from typing import Union, Tuple, Optional at the top of the file.
def detect_language_and_namespace(file_path: Path) -> "Union[Tuple[str, str], Tuple[None, None]]":| except: | ||
| continue |
There was a problem hiding this comment.
Using a bare except: is discouraged because it catches all exceptions, including SystemExit and KeyboardInterrupt, which can make the script difficult to terminate. It's better to catch specific exceptions you anticipate, like json.JSONDecodeError or IOError.
except (json.JSONDecodeError, IOError):
continue| def find_hardcoded_in_file(file_path: Path, file_type: str, patterns: list[tuple[str, str]], has_i18n: bool, include_console: bool) -> list[dict]: | ||
| issues = [] | ||
| try: | ||
| lines = file_path.read_text(encoding='utf-8', errors='ignore').splitlines() |
There was a problem hiding this comment.
Using errors='ignore' when reading files can hide encoding issues and lead to silent data corruption, potentially causing the checker to miss hardcoded strings. It's safer to use errors='replace', which substitutes problematic bytes with a placeholder, making it clear that an encoding error occurred without stopping the script.
lines = file_path.read_text(encoding='utf-8', errors='replace').splitlines()| except: | ||
| return issues |
There was a problem hiding this comment.
Using a bare except: is discouraged because it catches all exceptions, including SystemExit and KeyboardInterrupt, which can make the script difficult to terminate. It's better to catch specific exceptions you anticipate, such as IOError for file reading operations.
except IOError:
return issues| except: | ||
| continue |
There was a problem hiding this comment.
Using a bare except: is discouraged because it catches all exceptions, including SystemExit and KeyboardInterrupt. If the goal is to catch any potential error during the processing of a single file and continue with the next, it's better to explicitly catch Exception.
except Exception:
continueAdd new terminology reference file to standardize translations across the project. Update SKILL.md to include terminology section and update checklist to include glossary verification. This ensures consistent UI naming and prevents translation of product names and code identifiers.
1 participant
📝 Pull Request 描述 | Description
Add new skill for internationalization and localization best practices. Includes comprehensive documentation, best practices guide, and a Python script to detect hardcoded strings and verify locale completeness.
🎯 变更类型 | Change Type
🔗 相关 Issue | Related Issues
📋 变更内容 | Changes Made
主要变更 | Main Changes
技术细节 | Technical Details
🧪 测试 | Testing
测试环境 | Test Environment
测试步骤 | Test Steps
测试结果 | Test Results
📸 截图/录屏 | Screenshots/Recordings
变更前 | Before
变更后 | After
破坏性变更详情 | Breaking Changes Details
✅ 检查清单 | Checklist
代码质量 | Code Quality
测试 | Testing
文档 | Documentation
其他 | Others
📌 额外说明 | Additional Notes
🙏 致谢 | Acknowledgements
📖 提示 | Tips:
/cc @maintainers