-
Notifications
You must be signed in to change notification settings - Fork 549
Korean lang fix #1677
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
Korean lang fix #1677
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.
❌ Changes requested. Reviewed everything up to 246c18f in 3 minutes and 58 seconds
More details
- Looked at
411
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
11
drafted comments based on config settings.
1. apps/studio/src/i18n.ts:5
- Draft comment:
Confirm that 'kr' is the intended language code. Standard ISO code for Korean is 'ko'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
While technically correct that 'ko' is the ISO standard, this comment violates the rule about not asking authors to confirm their intentions. The code will work fine with 'kr' - it's just a key in an object. The actual translations would work regardless of the code used. This is more of an informational comment than a required change.
The use of non-standard language codes could cause confusion for future maintainers or make the codebase inconsistent with other international systems.
While using standard codes is good practice, this comment is phrased as a verification request rather than a clear action item, and the code will function correctly either way.
Delete this comment as it violates the rule about asking for confirmation and is more informational than actionable.
2. apps/studio/src/locales/kr/translation.json:1
- Draft comment:
Ensure translations are contextually natural and consistent with other language files. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. apps/studio/src/routes/editor/Toolbar/Terminal/RunButton.tsx:119
- Draft comment:
Good update to regex: check that including Korean characters in the regex is sufficient for all expected cases. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. apps/studio/src/i18n.ts:5
- Draft comment:
Korean translation is imported and added correctly. Consider using locale auto-detection instead of hard-coding 'en' as the default language if appropriate. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
5. apps/studio/src/locales/kr/translation.json:1
- Draft comment:
The new Korean translation file is comprehensive and contextually natural. Verify that key names match those in other locale files for consistency. - Reason this comment was not posted:
Confidence changes required:10%
<= threshold50%
None
6. apps/studio/src/routes/editor/LayersPanel/HelpDropdown.tsx:95
- Draft comment:
Mapping over the Language enum with Object.values is concise and scalable for adding new languages. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
7. apps/studio/src/routes/editor/Toolbar/Terminal/RunButton.tsx:120
- Draft comment:
The regex update correctly includes the Korean character range for adjusting button width based on language. Good update! - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
8. apps/studio/src/routes/editor/Toolbar/Terminal/RunButton.tsx:126
- Draft comment:
The tooltip text in getTooltipText() is hard-coded in English. For consistent internationalization, consider moving these strings to the translation files. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. packages/models/src/constants/language.ts:5
- Draft comment:
The addition of Korean in the Language enum and its display name is correctly implemented and consistent with other languages. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
10. apps/studio/src/i18n.ts:18
- Draft comment:
The language key 'kr' is used for Korean, but the standard ISO 639-1 code for Korean is 'ko'. Consider updating the key (and corresponding import path if needed) to 'ko' to maintain consistency with conventional language codes. - Reason this comment was not posted:
Marked as duplicate.
11. packages/models/src/constants/language.ts:5
- Draft comment:
The language code for Korean is typically 'ko' (ISO 639-1) rather than 'kr', which is commonly used as a country code. Please double-check and update this if it requires consistency with the other language codes. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_XhAtar4vlf0sqMJc
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@Kitenite |
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.
Awesome thanks @homebodify ! Please lmk if I missed anything :)
@Kitenite |
Description
Fix the contextually unnatural parts of the Korean text
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Add Korean language support with translations and UI adjustments for language selection.
translation.json
.i18n.ts
to include Korean language resources.HelpDropdown.tsx
to dynamically generate language options, including Korean.RunButton.tsx
to handle Korean characters in button width calculation.Language
enum andLANGUAGE_DISPLAY_NAMES
inlanguage.ts
.This description was created by
for 246c18f. It will automatically update as commits are pushed.