Skip to content
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

update canonicalizeLocale to handle 中文(chinese) #239

Merged
merged 7 commits into from
Aug 18, 2021

Conversation

oshi97
Copy link
Contributor

@oshi97 oshi97 commented Aug 18, 2021

J=SLAP-1526
TEST=auto,manual

unit tests
ran build with zh-hans-ch in locale config, saw that the generated source code used the correct locale in the page
tried building a page with a locale with >3 sections, got a jambo warning but build continued without changing the locale

@coveralls
Copy link

coveralls commented Aug 18, 2021

Coverage Status

Coverage increased (+1.0%) to 47.686% when pulling a7302c2 on dev/canonicalize-locale-handle-zhongwen into 2d76f4d on develop.

@oshi97
Copy link
Contributor Author

oshi97 commented Aug 18, 2021

was testing in the theme and this doesn't handle zh-Hans correctly (i.e. chinese w/ no region code)

src/utils/i18nutils.js Outdated Show resolved Hide resolved
@oshi97 oshi97 requested a review from cea2aj August 18, 2021 20:34
const numSections = localeCodeSections.length;
if (numSections === 1) {
return {};
} else if (numSections === 2 && language === 'zh') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we handle codes such as: zh_CH? Currently it would treat CH as a modifier when really its a region

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I missed this case

Copy link
Member

@cea2aj cea2aj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥🔥🔥

@oshi97 oshi97 merged commit 44b7f79 into develop Aug 18, 2021
@oshi97 oshi97 deleted the dev/canonicalize-locale-handle-zhongwen branch August 18, 2021 21:44
@oshi97 oshi97 mentioned this pull request Aug 24, 2021
oshi97 added a commit that referenced this pull request Aug 24, 2021
* Do not run the babel helper in Development Mode. (#224) (#225)
* Use npmlog for logging with colors (#228)
* Add Jambo build validation hook (#229)
* Added acceptance testing framework + tests 
* add --globs param to `jambo extract-translations` (#233) 
* Pass partials to templatedatavalidator (#237)
* `jambo upgrade` no longer deletes the themes folder if there is an error when git checkout-ing the new branch (#235)
* Update canonicalizeLocale to handle 中文(chinese) (#239)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants