Add zh-CN.json translations and respective compatibility checks via i18n tools#30378
Merged
azasypkin merged 5 commits intoelastic:masterfrom Feb 11, 2019
Merged
Conversation
…ations file compatibility check.
azasypkin
added
dev
Team:Core
Contributor
|
Pinging @elastic/kibana-platform |
azasypkin
commented
Feb 7, 2019
| "x-pack/plugins/infra/server/graphql/types.ts", | ||
| "x-pack/plugins/infra/server/lib/domains/log_entries_domain/log_entries_domain.ts" | ||
| ], | ||
| "translations": [ |
Contributor
Author
There was a problem hiding this comment.
note: we need to track these files somewhere for a compatibility checks and automatic fixes.
Contributor
💚 Build Succeeded |
azasypkin
changed the title
zh-CN.json translations and respective compatibility via i18n toolszh-CN.json translations and respective compatibility checks via i18n tools
Bamieh
approved these changes
Feb 11, 2019
| await readFileAsync(resolve(configPath)) | ||
| ); | ||
|
|
||
| for (const [namespace, path] of Object.entries(additionalConfig.paths || [])) { |
Contributor
There was a problem hiding this comment.
I think it is neater to merge defaults when defining additionalConfig instead of having || statements in the for statements
|
|
||
| /** | ||
| * Filters out custom paths based on the paths defined in config and that are | ||
| * known to contain 1i8n strings. |
Contributor
There was a problem hiding this comment.
Suggested change
| * known to contain 1i8n strings. | |
| * known to contain i18n strings. |
| const mockMakeDirAsync = jest.fn(); | ||
| jest.mock('./utils', () => ({ | ||
| // Jest typings don't define `requireActual` for some reason. | ||
| ...(jest as any).requireActual('./utils'), |
Contributor
There was a problem hiding this comment.
(ignore this; just a general note)
Weird it is in the DefinitelyTyped definition, maybe on a newer version.
Contributor
Author
There was a problem hiding this comment.
Yeah, we need to update our jest types at some point (and jest as well).
|
|
||
| const missingTranslations = difference(defaultMessagesIds, localizedMessagesIds); | ||
| if (!options.ignoreMissing && missingTranslations.length > 0) { | ||
| errorMessage += `\n${ |
Contributor
There was a problem hiding this comment.
Maybe moving the missingTranslations definition line inside the options.ignoreMissing condition is better to avoid evaluating for nothing?
Contributor
💚 Build Succeeded |
azasypkin
added a commit
to azasypkin/kibana
that referenced
this pull request
Feb 11, 2019
azasypkin
added a commit
to azasypkin/kibana
that referenced
this pull request
Feb 11, 2019
Contributor
Author
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #30143
The only non-dev thing that this PR includes is the file with translations (
zh-CN.json), the rest are changes in i18n dev tools that can help us to maintainzh-CN.jsonin master as compatible to the new changes as possible.List of notable changes:
zh-CN.jsonforward-ported from 6.7 branchi18n_checktoi18n_extractand introduced newi18n_checkbased oni18n_extractandi18n_integrate--ignore-unused,--ignore-missingand--ignore-incompatibleoptions to bothi18n_checkandi18n_integrate(see docs for more details)translationsfield to.i18nrc.jsonto know which files exactly we should use in compatibility checks--targetparameter fori18n_integrateto store all translations inside of a single file to supportx-pack/translations(without this option, translations file is split and distributed across plugin folders based on mappings defined in.i18nrc.json)On CI we'll run (
--ignore-missingto not complain about new non-translated labels):The command above will do the following checks:
translationsfrom.i18nrc.jsonand compare them to the messages extracted and validated at the step above and:2.1 Check for unused translations - if developer removed a label which has corresponding translation they will need to remove this label from translations file as well, semi-automatic for now (see below)
2.2 Check for incompatible translations - if developer, let's say removed or added new parameter to an existing string, they will need to remove this label from translations file.
To fix 2.1 and 2.2 (assuming changes are intentional), one should run the following command:
That is currently expanded into the following command under the hood (simplified):
We can tune defaults once we get feedback, let's see.
To integrate files we receive from translators right now, one can use the following command:
The
ignore-*options shouldn't be used when integrating into the same revision from whichen.jsonwas created otherwise we may miss some errors that were done during translation.