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.

Sign up for GitHub

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

Jump to bottom

move canvas interpreter to OSS #25711

Merged
merged 12 commits into from
Nov 20, 2018
Merged

move canvas interpreter to OSS #25711

merged 12 commits into from
Nov 20, 2018

Conversation

ppisljar
Copy link
Member

@ppisljar ppisljar commented Nov 15, 2018 •
edited by markov00
Loading

Summary

closes #23096
preparation for #23185

moves some parts of canvas to kibana core:

  • interpreter parse functions are moved to @kbn/interpreter
  • function and type registires are moved to @kbn/interpreter
  • all types are moved to @kbn/interpreter
  • clog common function is moved to @kbn/interpreter
  • socket, plugin and translate server routes are moved to the interpreter core_plugin
  • browser socket wrapper is moved to the interpreter core_plugin

qa: nothing should change functionally

Checklist

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

For maintainers

@ppisljar ppisljar added WIP Work in progress Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Nov 15, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@ppisljar ppisljar mentioned this pull request Nov 15, 2018
Closed
@ppisljar ppisljar force-pushed the ref/canvasInterpreter2OSS branch 5 times, most recently from e382cf4 to 626234a Compare November 16, 2018 09:06
@elastic elastic deleted a comment from elasticmachine Nov 16, 2018
@elastic elastic deleted a comment from elasticmachine Nov 16, 2018
@elastic elastic deleted a comment from elasticmachine Nov 16, 2018
@elastic elastic deleted a comment from elasticmachine Nov 16, 2018
@elastic elastic deleted a comment from elasticmachine Nov 16, 2018
@elastic elastic deleted a comment from elasticmachine Nov 16, 2018
@elastic elastic deleted a comment from elasticmachine Nov 16, 2018
@elastic elastic deleted a comment from elasticmachine Nov 16, 2018
@elastic elastic deleted a comment from elasticmachine Nov 16, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ppisljar ppisljar added review v7.0.0 v6.6.0 and removed WIP Work in progress labels Nov 16, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

markov00
markov00 approved these changes Nov 19, 2018
View reviewed changes
Copy link
Member

@markov00 markov00 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 on chrome/OSX and works correctly.

packages/kbn-interpreter/public/browser_registries.js Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

lukeelmers
lukeelmers approved these changes Nov 19, 2018
View reviewed changes
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.

Changes all LGTM!

rashidkpc
rashidkpc suggested changes Nov 19, 2018
View reviewed changes
Copy link
Contributor

@rashidkpc rashidkpc left a comment

Choose a reason for hiding this comment

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

Few questions and comments. Thanks for separating out the license and file moving commits!

packages/kbn-interpreter/common/lib/paths_registry.js Show resolved Hide resolved
packages/kbn-interpreter/public/browser_registries.js Show resolved Hide resolved
packages/kbn-interpreter/public/browser_registries.js Show resolved Hide resolved
packages/kbn-interpreter/server/server_registries.js Show resolved Hide resolved
packages/kbn-interpreter/server/server_registries.js Show resolved Hide resolved
src/core_plugins/interpreter/server/routes/plugins.js Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ppisljar ppisljar merged commit 43b1d00 into elastic:master Nov 20, 2018
@ppisljar ppisljar mentioned this pull request Nov 20, 2018
Merged
ppisljar added a commit to ppisljar/kibana that referenced this pull request Nov 20, 2018
@ppisljar
move canvas interpreter to OSS (elastic#25711)
9e9b93b
@ppisljar ppisljar deleted the ref/canvasInterpreter2OSS branch November 20, 2018 08:57
@ppisljar ppisljar removed the review label Nov 20, 2018
ppisljar added a commit that referenced this pull request Nov 20, 2018
@ppisljar
[6.x] move canvas interpreter to OSS (#25711) (#25921)
d76c0ae
@mistic mistic mentioned this pull request Nov 21, 2018
Closed
w33ble added a commit to w33ble/kibana that referenced this pull request Nov 21, 2018
@w33ble
chore: revert elastic#25711
cdfc7dd
@w33ble w33ble mentioned this pull request Nov 21, 2018
Merged
@w33ble
Copy link
Contributor

w33ble commented Nov 21, 2018

Reverted in #26054 / 4f1ca7e, since this broke the build. There are tests in place now to avoid breaking the build again too.

w33ble added a commit that referenced this pull request Nov 21, 2018
@w33ble
Fix: revert and enable canvas tests (#26054)
4f1ca7e 
## Summary

CI was broken when enabling functional smoke tests via #25262. The problem is that the Kibana build is broken in master...

I think #25711 broke it, which was merged in between #25262 passing and it being merged (and then CI breaking). So this PR reverts that PR and enabled canvas smoke tests again to see if CI will pass.

- Revert #25711
- Revert 7ae0d28
@ppisljar ppisljar restored the ref/canvasInterpreter2OSS branch November 22, 2018 04:25
@ppisljar ppisljar mentioned this pull request Nov 22, 2018
Closed
7 tasks
@mistic mistic mentioned this pull request Nov 24, 2018
Closed
@jgowdyelastic jgowdyelastic mentioned this pull request Jan 7, 2019
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clintandrewhall clintandrewhall clintandrewhall left review comments

@rashidkpc rashidkpc rashidkpc requested changes

@markov00 markov00 markov00 approved these changes

@lukeelmers lukeelmers lukeelmers approved these changes

Assignees
No one assigned
Labels
Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure v6.6.0 v7.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Moving interpreter engine into Kibana
7 participants
@ppisljar @elasticmachine @w33ble @clintandrewhall @rashidkpc @markov00 @lukeelmers