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

Migration: Separate legacy and index entrypoint #54124

Merged
merged 12 commits into from
Jan 21, 2020

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Jan 7, 2020

This PR separates the shim plugin initialization from the index module of the shimmed Kibana App plugins which also export types or utilities.

If index.ts both initializes the plugin and re-exports static code, the whole plugin will always be initialized even if just a constant is used from the file (this is especially relevant in tests). This PR separates the two concerns by introducing a legacy and an index file per shimmed plugin - legacy initializes the shimmed plugin, index just re-exports static code.

Additionally this PR switches to the NP usageCollection plugin to initialize the telemetry functions in the home plugin.

@flash1293 flash1293 added Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes v7.6.0 v8.0.0 labels Jan 7, 2020
@flash1293 flash1293 marked this pull request as ready for review January 8, 2020 16:40
@flash1293 flash1293 requested a review from a team January 8, 2020 16:40
@flash1293 flash1293 added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Jan 8, 2020
@elasticmachine
Copy link
Contributor

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

@kertal
Copy link
Member

kertal commented Jan 14, 2020

@elasticmachine merge upstream

import { DiscoverPlugin } from './plugin';

// Core will be looking for this when loading our plugin in the new platform
export const plugin = (context: PluginInitializerContext) => {
Copy link
Member

Choose a reason for hiding this comment

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

Just a quick question: shouldn't plugin stay in index.ts be imported in legacy.ts? since "core will be looking for this when loading our plugin in the new platform". it that's the case index.ts would already be ready for NP, wouldn't it be?
however it's not a big change to be done when the cutover is done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are completely right, changed that. A small thing, but will make final migration easier.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@kertal kertal self-requested a review January 21, 2020 13:43
Copy link
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 👍 , didn't test now.

@flash1293 flash1293 merged commit c88aa5a into elastic:master Jan 21, 2020
@flash1293 flash1293 deleted the migration/separate-legacy-index branch January 21, 2020 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants