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

Shape data plugin into standard shim form #42238

Merged
merged 17 commits into from
Aug 1, 2019

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Jul 30, 2019

Summary

Shape data plugin into standard shim form.

MIGRATION.md updated to match these changes. #42338

Dev Docs

New plugin shape

  • data/public/index.ts - export types & static code
  • data/public/plugin.ts - defines the plugin services
  • data/public/legacy.ts - import any legacy dependencies we have, and shim them into our plugin
  • data/public/setup.ts - compatibility with 7.3 docs
  • data/public/shim/legacy_dependencies_plugin
  • data/public/shim/legacy_module

Loading plugin instance

// Old
import { data } from 'plugins/data/setup';

// New way added
import { setup as data } from 'plugins/data/legacy';

// The previous way is still supported!
import { data } from 'plugins/data/setup';

Loading legacy directives

Moved all remaining angular directives (from the filter service, others were already deleted) into the shim folder.

// Up to 7.3
import { data } from 'plugins/data/setup';	
data.filter.loadLegacyDirectives();	

// New, as a result of the new legacy dependencies plugin
import 'plugins/data/legacy';

Loading react components

// Up to 7.3
import { data } from 'plugins/data/setup';	
const QueryBar = data.ui.QueryBar;

<QueryBar>...

// New 
import { QueryBar } from 'plugins/data';	

<QueryBar>...

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@lizozom lizozom requested a review from a team July 30, 2019 10:37
@lizozom lizozom added the release_note:skip Skip the PR/issue when compiling release notes label Jul 30, 2019
@lizozom lizozom self-assigned this Jul 30, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@lizozom lizozom requested a review from a team July 30, 2019 11:23
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom
Copy link
Contributor Author

lizozom commented Jul 30, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

Overall makes sense! Added a couple notes, all pretty minor.

In this PR, do you think you could also update the legacy / new platform chart in the migration guide to ensure it has the most accurate info based on these changes? The platform team has asked that we try to keep this up-to-date as it helps people understand where to find things.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lizozom
Copy link
Contributor Author

lizozom commented Jul 31, 2019

@alexwizp #42159 depends on this PR.

@lizozom
Copy link
Contributor Author

lizozom commented Jul 31, 2019

@lukeelmers docs updated #42338

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@alexwizp alexwizp left a comment

Choose a reason for hiding this comment

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

TSVB changes looks good to me. Thanks

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

LGTM! And thanks for opening #42338!

@elasticmachine
Copy link
Contributor

💔 Build Failed

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 v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants