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

Expose Vis on the contract as it requires visTypes #56968

Merged
merged 4 commits into from
Feb 15, 2020

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Feb 6, 2020

Summary

Dev Docs

In most of the places Vis used as a type, but in couple places it is used as a class.
At the moment Vis as a class is not stateless, as it depends on visTypes. As it is not stateless, Vis class was removed from public exports and exposed on visualisations contract instead:

new visualizationsStart.Vis(....);

Vis as interface still can be imported as:

import { Vis } from '../../../../../core_plugins/visualizations/public';

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@ppisljar ppisljar mentioned this pull request Feb 6, 2020
11 tasks
});
export const start = pluginInstance.start(npStart.core, {
...npStart.plugins,
visualizations: visualizationsStart,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ppisljar, is this correct way to pass visualizations until is not migrated to NP?

@elasticmachine
Copy link
Contributor

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

@Dosant Dosant added v7.7.0 v8.0.0 Feature:NP Migration Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. labels Feb 6, 2020
@Dosant Dosant marked this pull request as ready for review February 6, 2020 13:41
@Dosant Dosant requested a review from a team February 6, 2020 13:41
@Dosant Dosant requested a review from a team as a code owner February 6, 2020 13:41
@Dosant Dosant requested a review from ppisljar February 6, 2020 13:41
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Kibana app changes LGTM, didn't test locally. The way how visualizations plugin is passed into the discover shim looks good.

@Dosant Dosant added the review label Feb 7, 2020
Copy link
Member

@ppisljar ppisljar 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

@Dosant
Copy link
Contributor Author

Dosant commented Feb 14, 2020

Retest

@Dosant
Copy link
Contributor Author

Dosant commented Feb 14, 2020

@elasticmachine merge upstream

@Dosant
Copy link
Contributor Author

Dosant commented Feb 15, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@Dosant Dosant merged commit e370df2 into elastic:master Feb 15, 2020
@Dosant Dosant deleted the dev/vis-to-np/expose-vis-on-plugin branch February 15, 2020 11:46
Dosant added a commit to Dosant/kibana that referenced this pull request Feb 15, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Dosant added a commit that referenced this pull request Feb 15, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 17, 2020
…re/files-and-filetree

* 'master' of github.com:elastic/kibana: (139 commits)
  Move Ace XJSON lexer-rules, worker and utils to es_ui_shared (elastic#57563)
  [Upgrade Assistant] Fix filter deprecations search filter (elastic#57541)
  [ML] New Platform server shim: update indices routes (elastic#57685)
  Bump redux dependencies (elastic#53348)
  [Index management] Client-side NP ready (elastic#57295)
  change id of x-pack event_log plugin to eventLog (elastic#57612)
  [eventLog] get kibana.index name from config instead of hard-coding it (elastic#57607)
  revert
  allow using any path to generate
  fixes ui titles (elastic#57535)
  Fix login redirect for expired sessions (elastic#57157)
  Expose Vis on the contract as it requires visTypes (elastic#56968)
  [SIEM][Detection Engine] Fixes queries to ignore errors when signals index is not present
  [Remote clusters] Migrate client-side code out of legacy (elastic#57365)
  Fix failed test reporter for SIEM Cypress use (elastic#57240)
  skip flaky suite (elastic#45244)
  update chromedriver to 80.0.1 (elastic#57602)
  change slack action to only report on whitelisted host name (elastic#57582)
  [kbn/optimizer] throw errors into stream on invalid completion (elastic#57735)
  moving visualize/utils to new platform (elastic#56650)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. review v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants