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

Fix i18n default formats injection into en.json #20929

Merged

Conversation

LeanidShutau
Copy link
Contributor

Bug: en.json generated by default messages extraction tool doesn't contain formats object.

#19620 imports { formats } from '@kbn/i18n' to inject it into en.json, but @kbn/i18n package structure was changed in #20513, so formats is undefined now.

How to test

Copy https://gist.github.com/LeanidShutau/6026b1ab9cc7e7b9d457676616abeb68 to src/core_plugins/kibana/common.

Before fix

Run node scripts/extract_default_translations src/core_plugins/kibana.
Open src/core_plugins/kibana/translations/en.json.
formats property is missing in en.json.

After fix

Run node scripts/extract_default_translations src/core_plugins/kibana.
Open src/core_plugins/kibana/translations/en.json.
Default formats object is injected into en.json.

@LeanidShutau LeanidShutau added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Project:i18n labels Jul 18, 2018
@LeanidShutau LeanidShutau self-assigned this Jul 18, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link

@yankouskia yankouskia left a comment

Choose a reason for hiding this comment

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

Good catch!

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM, just a minor nit. Thanks!


const extractedObject = JSON5.parse(extractedJSONBuffer.toString());

expect(extractedObject).toHaveProperty('formats');
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe use toMatchSnapshot instead of toHaveProperty to make sure it's a valid formats object we expect?

@LeanidShutau LeanidShutau force-pushed the default-messages-formats-hotfix branch from 771e9f9 to d12d173 Compare July 19, 2018 09:09
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@LeanidShutau LeanidShutau merged commit a091cf0 into elastic:master Jul 19, 2018
@LeanidShutau LeanidShutau deleted the default-messages-formats-hotfix branch July 19, 2018 11:49
LeanidShutau added a commit to LeanidShutau/kibana that referenced this pull request Aug 3, 2018
* Fix i18n default formats injection into en.json

* Use snapshots for tests
LeanidShutau added a commit that referenced this pull request Aug 3, 2018
* Fix i18n default formats injection into en.json

* Use snapshots for tests
@LeanidShutau
Copy link
Contributor Author

6.x/6.5: ff39ac1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported bug Fixes for quality problems that affect the customer experience Project:i18n Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v6.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants