Skip to content

[Vega] Shim new platform - cleanup vega_visualization dependencies#53605

Merged
alexwizp merged 4 commits intoelastic:masterfrom
alexwizp:vega_cleanup
Dec 23, 2019
Merged

[Vega] Shim new platform - cleanup vega_visualization dependencies#53605
alexwizp merged 4 commits intoelastic:masterfrom
alexwizp:vega_cleanup

Conversation

@alexwizp
Copy link
Copy Markdown
Contributor

@alexwizp alexwizp commented Dec 19, 2019 •
edited
Loading

Part of #38247

Summary

What was done in this PR:

  • Cleanup vega_visualization.js
    • remove all imports from 'ui/*'
  • Cleanup vega_request_handler.ts
    • remove ui/timefilter dependency
  • services.ts file was created to pass Start dependencies into Vega visualization

Checklist

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

For maintainers

@alexwizp alexwizp changed the title Cleanup Vega Vis [Vega] Shim new platform - cleanup vega_visualization dependencies Dec 21, 2019
@alexwizp alexwizp requested a review from flash1293 December 21, 2019 14:56
@elastic elastic deleted a comment from elasticmachine Dec 21, 2019
@alexwizp alexwizp removed the request for review from flash1293 December 21, 2019 15:33
@elastic elastic deleted a comment from kibanamachine Dec 23, 2019
@alexwizp alexwizp self-assigned this Dec 23, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 23, 2019

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

@alexwizp alexwizp added review Feature:Vega Vega visualizations release_note:skip Skip the PR/issue when compiling release notes Feature:NP Migration labels Dec 23, 2019
@alexwizp alexwizp marked this pull request as ready for review December 23, 2019 09:07
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Dec 23, 2019

💚 Build Succeeded

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

@alexwizp
Copy link
Copy Markdown
Contributor Author

alexwizp commented Dec 23, 2019

@elasticmachine merge upstream

streamich
streamich approved these changes Dec 23, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

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

src/legacy/core_plugins/vis_type_vega/public/services.ts
Comment on lines +27 to +32
export const [getNotifications, setNotifications] = createGetterSetter<NotificationsStart>(
'Notifications'
);

export const [getSavedObjects, setSavedObjects] = createGetterSetter<SavedObjectsStart>(
'SavedObjects'
Copy link
Copy Markdown
Contributor

@streamich streamich Dec 23, 2019

Choose a reason for hiding this comment

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

Maybe you could use createKibanaUtilsCore from kibana_utils here.

Copy link
Copy Markdown
Contributor

@lizozom lizozom Dec 23, 2019

Choose a reason for hiding this comment

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

I think I'd make an official best practice out of either option

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Dec 23, 2019

💚 Build Succeeded

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

lizozom
lizozom approved these changes Dec 23, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Dec 23, 2019

💚 Build Succeeded

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

@alexwizp alexwizp merged commit 6ff8931 into elastic:master Dec 23, 2019
alexwizp added a commit to alexwizp/kibana that referenced this pull request Dec 23, 2019
@alexwizp @elasticmachine
[Vega] Shim new platform - cleanup vega_visualization dependencies (e…
827e8fc 
…lastic#53605)

* Cleanup Vega Vis

* fix PR comments

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@alexwizp alexwizp mentioned this pull request Dec 23, 2019
Merged
alexwizp added a commit that referenced this pull request Dec 23, 2019
@alexwizp @elasticmachine
[Vega] Shim new platform - cleanup vega_visualization dependencies (#…
baebbeb 
…53605) (#53772)

* Cleanup Vega Vis

* fix PR comments

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 Dec 30, 2019
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into feature/conso…
13e7c02 
…le-saved-objects

* 'master' of github.com:elastic/kibana: (250 commits)
  Allow chromeless applications to render via non-/app routes (elastic#51527)
  Add server rendering service to enable standalone route rendering (elastic#52161)
  Possibility to filter when testing scripted fields (elastic#35379) (elastic#44220)
  Update maps telemetry mappings to account for recent updates (elastic#53803)
  [Maps] Only show legend when layer is visible (elastic#53781)
  remove use of experimental fs.promises api (elastic#53346)
  [APM] Add log statements for flaky test (elastic#53775)
  [APM] Transaction page throws unhandled exception if transactions doesn't have  `http.request` (elastic#53760)
  Licensing plugin functional tests (elastic#53580)
  [Lens] Disable saving visualization until there are no changes to the document (elastic#52982)
  [Monitoring] Added safeguard for some EUI components (elastic#53318)
  [Vega] Shim new platform - cleanup vega_visualization dependencies (elastic#53605)
  Display changed field formats without requiring hard page refresh. (elastic#53746)
  Upgrade EUI to v17.3.1 (elastic#53655)
  [APM] Fix missing apm indicies (elastic#53541)
  Disable inspector for timelion (elastic#53747)
  Clean up search servie (elastic#53701)
  [Dashboard] Grid: removing double handler (elastic#53707)
  Remove SavedObjectRegistryProvider from codebase (elastic#53455)
  Move ui/courier into data shim plugin (elastic#52359)
  ...
@alexwizp alexwizp deleted the vega_cleanup branch January 4, 2020 08:19
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 8, 2020
@alexwizp @elasticmachine @jkelastic
[Vega] Shim new platform - cleanup vega_visualization dependencies (e…
1707e5a 
…lastic#53605)

* Cleanup Vega Vis

* fix PR comments

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukeelmers lukeelmers Awaiting requested review from lukeelmers

@flash1293 flash1293 Awaiting requested review from flash1293

2 more reviewers

@lizozom lizozom lizozom approved these changes

@streamich streamich streamich approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alexwizp alexwizp

Labels

Feature:NP Migration Feature:Vega Vega visualizations release_note:skip Skip the PR/issue when compiling release notes review v7.6.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@alexwizp @elasticmachine @kibanamachine @lizozom @streamich