「AppArch」Interpreter 👉 New Platform #39329
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
streamich
added
the
release_note:skip
|
Error when running Contents of test bundle:
Above error is caused when NP plugin imports from |
|
Pinging @elastic/kibana-app-arch |
💔 Build Failed |
💔 Build Failed |
|
Please re-request review once this is ready |
💚 Build Succeeded |
💚 Build Succeeded |
💔 Build Failed |
mshustov
reviewed
Jun 27, 2019
|
|
||
| export function interpreterProvider(config) { |
There was a problem hiding this comment.
another problem with any - I often don't understand it is just a temporary mock or I can use any for real
This was referenced Jun 27, 2019
💚 Build Succeeded |
streamich
changed the title
spalger
approved these changes
Jul 1, 2019
mshustov
approved these changes
Jul 2, 2019
rudolf
requested changes
Jul 2, 2019
| * under the License. | ||
| */ | ||
|
|
||
| export const mockNewPlatformBackdoor = () => { |
There was a problem hiding this comment.
The name and location of this file suggests that it's a generic New Platform mock so other developers might also use this in their tests although the plugins.data mock is very specific to your unit tests. We'd rather want them to use #39351
Can you move this mock file into your plugin so that the code is more local?
Optional: Use / adapt #39351 for your tests. You'll have to add an empty plugins mock to https://github.com/elastic/kibana/pull/39351/files#diff-a0d66a99cc197c9ae8a0956f8df3bbdeR28
and then in your tests:
import { pluginsMock } from '../../../../../../../../core/public/ui_new_platform.test.mocks';
pluginsMock.mockReturnValue({
data: {
expressions: {...}
} There was a problem hiding this comment.
Created PR #40123 to address this.
This was referenced Jul 2, 2019
streamich
added a commit
that referenced
this pull request
Jul 8, 2019
* 「AppArch」Interpreter 👉 New Platform (#39329) * feat: 🎸 set-up NP data plugin * refactor: 💡 move interpreter functions registry to NP * refactor: 💡 move interpreter renderer registry to NP plugin * refactor: 💡 move interpreter typesRegistry to NP * refactor: 💡 move interpreter types to NP * chore: 🤖 import typeRegistry from NP and change TS type folder * refactor: 💡 move interpreter expression types to NP * refactor: 💡 move rest of interpreter common folder to NP plugin * fix: 🐛 fix TypeScript errors * test: 💍 improve typings and test mocks * refactor: 💡 make Interpreter internal registry impl private * test: 💍 inline NP backdoor mock creation in test suites * chore: 🤖 change @kbn/interpreter import paths to try fix errors * fix: 🐛 improve core Plugin interfaces * feat: 🎸 add stop() lifecycle to NP data plugins * refactor: 💡 move interpreter into expressions service data NP * refactor: 💡 inline Registry @kbn/interpreter class * refactor: 💡 remove dependency on @kbn/interpreter in data pub * refactor: 💡 move interpreter common dir into expressions dir * fix: 🐛 use TS types in kibana_context * feat: 🎸 add types suggested in PR review * feat: 🎸 add semantic interpreter registration functions * refactor: 💡 use require for all @kbn/interpreter imports * test: 💍 add Karma test mocks, thx @spalger 🙏 * docs: ✏️ update Core docs * test: 💍 add Sinon stubs for registries * chore: 🤖 change import syntax in hopes CI will work * chore: 🤖 set App Architecture as owners of data plugin * docs: ✏️ add README * chore: 🤖 change import in hopes to fix optimizer * fix: 🐛 make stop() plugin life-cycle optional * docs: ✏️ update Core API docs * test: 💍 remove unnecessary Jest mock * chore: 🤖 don't import from deeply inside a plugin * refactor: 💡 try different interpreter import * fix: 🐛 fix Karma mocking * fix: 🐛 fix TypeScript type imports * test: 💍 fix broken test * fix: 🐛 use "kibana" version to make it work everywhere * style: 💄 fix linter error
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This moves about half of the Interpreter plugin to new platform.
Interpreter has two major parts: (1) registries; (2) the
run()function. This PR moves the registries to the NP, a subsequent PR will move therunfunction.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsUnit or functional tests were updated or added to match the most common scenariosThis was checked for keyboard-only and screenreader accessibilityFor maintainers
This was checked for breaking API changes and was labeled appropriatelyThis includes a feature addition or change that requires a release note and was labeled appropriately