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

Allow custom paths in plugin generator #57766

Merged
merged 16 commits into from
Feb 19, 2020

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Feb 16, 2020

Summary

Follow up for #55281
Partially resolves #56652

  • Allow setting custom paths for plugins, to support creating plugins in src/plugins and x-pack/plugins (or elsewhere).
  • Removes the -i argument (as it is now redundant)
  • Adds translation support

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@lizozom lizozom marked this pull request as ready for review February 17, 2020 11:08
@lizozom lizozom added Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:AppArch labels Feb 17, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@lizozom
Copy link
Contributor Author

lizozom commented Feb 17, 2020

@elasticmachine merge upstream

@lizozom
Copy link
Contributor Author

lizozom commented Feb 17, 2020

@elasticmachine merge upstream

Liza K added 2 commits February 17, 2020 14:08
Content of translation jsons
Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

Left some minor comments for i18n to work in the generated plugin. To ensure that the plugin i18n is working you can run the following commands:

Extract i18n labels from the plugin:

node scripts/i18n_extract --output-dir ./plugins/myPlugin --include-config ./plugins/myPlugin/.i18nrc.json

This should generate an en.json in kibana root with the i18n.translate labels in the plugin.

Checking valid i18n usage

node scripts/i18n_check --fix --include-config ./plugins/myPlugin/.i18nrc.json

This should fix any fixable i18n issues in the plugin and error our on unfixable errors (similar to eslint behavior)

Both scripts are ran from kibana root. We also need to update our docs https://www.elastic.co/guide/en/kibana/current/development-plugin-localization.html to start referring to ./plugins/ instead of ../kibana-extra/.

@lizozom
Copy link
Contributor Author

lizozom commented Feb 17, 2020

@Bamieh I addressed the fixes
However,
node scripts/i18n_extract --output-dir ./plugins/myPlugin --include-config ./plugins/myPlugin/.i18nrc.json
Runs on the entire project, making it very slow. Is there a way to run it on the plugin only?

@Bamieh
Copy link
Member

Bamieh commented Feb 17, 2020

@lizozom You are correct, the path flag must be added to get the desired result we're after.

node scripts/i18n_extract --output-dir ./plugins/myPlugin --include-config ./plugins/myPlugin/.i18nrc.json --path=./plugins/myPlugin/

Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

i18n stuff LGTM. thanks!

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Tried to create plugin in examples folder. It worked 👍

Minor issue I bumped into:
I answered /examples for the path question and it errored saying folder doesn't exist. examples worked. But I think /examples should also work.

@lizozom
Copy link
Contributor Author

lizozom commented Feb 19, 2020

@elasticmachine merge upstream

Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

Can we add a test to the existing integration_tests for this?

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@lizozom lizozom merged commit e1fa139 into elastic:master Feb 19, 2020
@joshdover
Copy link
Contributor

Can we add a test to the existing integration_tests for this?

@lizozom Is this going to be done in a follow up?

mbondyra added a commit to mbondyra/kibana that referenced this pull request Feb 20, 2020
* master: (136 commits)
  [Visualize] Remove legacy appState in visualize (elastic#57330)
  Use static time for tsvb rollup test (elastic#57701)
  [SIEM] Fix ResizeObserver polyfill (elastic#58046)
  [SIEM][Detection Engine] Fixes return codes where some were rule_id instead of id
  skip flaky suite (elastic#56816)
  skip flaky suite (elastic#58059)
  skip flaky suite (elastic#45348)
  migrates notification server routes to NP (elastic#57906)
  Moved all of the show/hide toggles outside of ordered lists. (elastic#57163)
  [APM] NP Migration - Moves plugin server files out of legacy (elastic#57532)
  [Maps][Telemetry] Migrate Maps telemetry to NP (elastic#55055)
  Embeddable add panel examples (elastic#57319)
  Fix useRequest to support query change (elastic#57723)
  Allow custom paths in plugin generator (elastic#57766)
  [SIEM][Case] Merge header components (elastic#57816)
  [ML] New Platform server shim: update job audit messages routes (elastic#57925)
  [kbn/optimizer] emit success event from reducer when all bundles cached (elastic#57945)
  [APM] Don’t include UI filters when fetching a specific transaction (elastic#57934)
  Upgrade yargs (elastic#57720)
  skip flaky suite (elastic#57762) (elastic#57997) (elastic#57998)
  ...

# Conflicts:
#	src/plugins/advanced_settings/public/management_app/components/field/__snapshots__/field.test.tsx.snap
#	src/plugins/advanced_settings/public/management_app/components/field/field.tsx
#	x-pack/plugins/translations/translations/ja-JP.json
#	x-pack/plugins/translations/translations/zh-CN.json
lizozom pushed a commit to lizozom/kibana that referenced this pull request Feb 24, 2020
* Allow custom paths

* Add translation file

* Fix jest test

* Added more tests

* Update docs
Content of translation jsons

* Leave only one translation file

* generate default translations file

* Simplify i18n setup

* Code review changes

Co-authored-by: Elastic Machine <[email protected]>
lizozom pushed a commit that referenced this pull request Feb 24, 2020
* Allow custom paths

* Add translation file

* Fix jest test

* Added more tests

* Update docs
Content of translation jsons

* Leave only one translation file

* generate default translations file

* Simplify i18n setup

* Code review changes

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin generator translations and external deps
6 participants