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.

Sign up for GitHub

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

Jump to bottom

[i18n] NPM publish i18ntokens.json #4771

Merged
merged 5 commits into from
May 4, 2021
Merged

[i18n] NPM publish i18ntokens.json #4771

merged 5 commits into from
May 4, 2021

Conversation

afharo
Copy link
Member

@afharo afharo commented Apr 30, 2021 •
edited
Loading

Summary

Publishing the i18ntokens.json to NPM will help any downstream users to identify all the keys they need to translate in their projects and, potentially, create tests in their CI to ensure that all tokens are covered before any release.

To achieve that, this PR moves the generation of i18ntokens.json to the root of the project instead of src-docs/src/. It also moves i18ntokens_changelog.json because it kind of makes sense that they are together.

Running the doc server locally, http://0.0.0.0:8030/#/package/i18n-tokens it still works, and npm publish --dry-run shows that the files are included in the NPM bundle now.

Related to elastic/kibana#74657.

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs and playground toggles
  • Added documentation
  • Checked Code Sandbox works for the any docs examples
  • Added or updated jest tests
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4771/

@afharo afharo marked this pull request as ready for review April 30, 2021 13:28
@afharo afharo mentioned this pull request Apr 30, 2021
Closed
@thompsongl thompsongl self-requested a review April 30, 2021 13:49
thompsongl
thompsongl reviewed May 3, 2021
View reviewed changes
Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Thanks, @afharo!

I checked out this branch and ran update_token_changelog, and the result doesn't look correct. Basically, it looks like every token was seen as "added" and as a result is duplicated in the latest version.

@afharo
Copy link
Member Author

afharo commented May 4, 2021

oops! Good catch! Let me double-check that, and I'll get back to you!

@afharo afharo requested a review from thompsongl May 4, 2021 08:55
@afharo
Copy link
Member Author

afharo commented May 4, 2021

@thompsongl the fix is in this commit: afa0831 (#4771). I was missing await when calling the new method (I'm so used to relying on TS to shout out about this 😅).
Thanks again for spotting this issue!

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4771/

thompsongl
thompsongl approved these changes May 4, 2021
View reviewed changes
Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

LGTM!
Confirmed that both i18n token files are included in the npm package, the update_token script works as expected, and that the files are accessible from @elastic/eui/[file]

CHANGELOG.md Outdated Show resolved Hide resolved
@afharo @thompsongl
Update CHANGELOG.md
22e237f 
Co-authored-by: Greg Thompson <thompson.glowe@gmail.com>
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4771/

@thompsongl thompsongl merged commit f5fadd9 into elastic:master May 4, 2021
@afharo afharo deleted the i18n/npm-publish-i18ntokens.json branch May 4, 2021 15:32
cchaos pushed a commit that referenced this pull request May 4, 2021
@afharo @thompsongl
[i18n] NPM publish i18ntokens.json (#4771)
0ac8774 
* [i18n] NPM publish `i18ntokens.json`

* Add entry in changelog

* Fix missing await

* Update CHANGELOG.md

Co-authored-by: Greg Thompson <thompson.glowe@gmail.com>

Co-authored-by: Greg Thompson <thompson.glowe@gmail.com>
@thompsongl thompsongl mentioned this pull request May 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thompsongl thompsongl thompsongl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@afharo @kibanamachine @thompsongl