Skip to content

Move local application service into Kibana platform#50661

Merged
kertal merged 20 commits intoelastic:masterfrom
flash1293:kibana-app-registry-in-np
Nov 22, 2019
Merged

Move local application service into Kibana platform#50661
kertal merged 20 commits intoelastic:masterfrom
flash1293:kibana-app-registry-in-np

Conversation

@flash1293
Copy link
Copy Markdown
Contributor

This PR moves the registry part of the local application service of the Kibana app plugin into the Kibana platform. By doing this apps registering into the local application service can already move to the new platform even when they have to be rendered without reload when switching from dashboard or visualize.

The dev tools app is already legacy-dependency free and is also moved over in this PR except for the

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@flash1293 flash1293 marked this pull request as ready for review November 15, 2019 19:38
@flash1293 flash1293 added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Nov 15, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@flash1293 flash1293 requested review from a team and kertal November 15, 2019 19:39
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

instance.start(npStart.core, {
newPlatformDevTools: npStart.plugins.devTools,
});
if (npStart.plugins.devTools.getSortedDevTools().length === 0) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@restrry qq: What's the convention for plugin naming? There is feature_catalogue, but uiActions

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

all legacy plugins use snake_case for naming https://github.com/elastic/kibana/tree/master/src/legacy/core_plugins

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@streamich do you want me to create an issue to fix that for ui_actions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@restrry Ah, maybe I wasn't clear. I was talking about the id of the plugin. The ui_actions/uiActions plugin uses snake case for the directory, but camel case for the id.

Copy link
Copy Markdown
Contributor

@mshustov mshustov Nov 19, 2019

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

@mshustov mshustov Dec 8, 2019

Choose a reason for hiding this comment

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

created #52190

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@flash1293 flash1293 requested a review from a team November 20, 2019 11:29
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@flash1293
Copy link
Copy Markdown
Contributor Author

Jenkins, test this.

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Copy link
Copy Markdown
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

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

I get a warning when I open the help menu in Lens:

React Intl] Could not find required intl object. <IntlProvider> needs to exist in the component ancestry. Using default message as fallback.

Copy link
Copy Markdown
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

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

The Lens code changes LGTM, but I did find an I18n warning with them.

@kertal kertal requested a review from wylieconlon November 21, 2019 10:08
Copy link
Copy Markdown
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM ,tested locally with Chrome

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@kertal
Copy link
Copy Markdown
Member

kertal commented Nov 22, 2019

@wylieconlon since the warning is gone, you're good from lens side, merging this ? thx

Copy link
Copy Markdown
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

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

Lens changes LGTM, did not re-test locally

@kertal kertal merged commit 8af09fa into elastic:master Nov 22, 2019
kertal pushed a commit to kertal/kibana that referenced this pull request Nov 22, 2019
* move local application service registry to new platform

* move dev tools app itself

* fix i18n

* make sure legacy dev tools are imported

* rename dev tools plugin
kertal added a commit that referenced this pull request Nov 25, 2019
…51499)

* Move local application service into Kibana platform (#50661)

* move local application service registry to new platform

* move dev tools app itself

* fix i18n

* make sure legacy dev tools are imported

* rename dev tools plugin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Dev Tools Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants