Closed
Conversation
Contributor
|
Pinging @elastic/kibana-app-arch |
This comment has been minimized.
This comment has been minimized.
042641b to
4145526
Compare
This comment has been minimized.
This comment has been minimized.
4145526 to
0cf6e19
Compare
This comment has been minimized.
This comment has been minimized.
Closed
Contributor
💚 Build Succeeded |
lizozom
approved these changes
Jul 4, 2019
Contributor
lizozom
left a comment
There was a problem hiding this comment.
In general, I love the separation between index \ plugin \ setup
I think that this should be applied immediately to the vis_markdown plugin as well, to see how it works.
And also to make sure that EPAM work with correct references.
| * Expressions Service | ||
| * @internal | ||
| */ | ||
| export class ExpressionsService { |
Contributor
There was a problem hiding this comment.
A bit unrelated but shouldn't we have registerFunction here?
I saw @ppisljar added it to expressions on markdown vis.
| const coreSetup: CoreSetup = npSetup.core; | ||
|
|
||
| // plugin dependency shims | ||
| const pluginsSetup: DataSetupPlugins = {}; |
This was referenced Jul 8, 2019
Contributor
Author
|
Going to go ahead and close this now that we've aligned on various conventions -- we can do the cleanup over the course of other refactorings that are in progress |
7 tasks
This file contains hidden or 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
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
Working to make a clear separation between shim code & "new platform" code, so that at the end of the migration process we can just delete the shims. Also trying to make everything consistent based on the most recent conventions we have had, so that the data plugin can serve as an example for future plugins and we can point other teams to it as the recommended way of doing things.
Proposed Conventions
These changes imply a few conventions I'd like to propose (which could ultimately be added to #39542 that @joshdover started, once we get a consensus here):
server/andpublic/directory has:index.tswhich exports a function namedplugin(already required by NP)plugin.tsin a separate file which contains the plugin definition (already a recommended convention for NP).NameSetup, andNameStart, and would define interfaces for it's own plugin dependencies, named asNameSetupPluginsandNameStartPluginsPlugintype as wellDataPublicPlugin(server would beDataServerPlugin). Stole this idea from @streamich 😉setup.ts, which was originally added by @lizozom. This file would serve exclusively as a shim that gets imported by other plugins.plugin.tsfile can remain a "clean" representation of how it will look in the new platform. In a perfect world, these shim files can simply be deleted when things move to new platform and everything Just Works(TM).setup.tsshim files!) to get the setup contract for our shim plugin.start.tsshim file would get created.npSetupfromui/new_platformto inject core dependencies.I'm working on a proposed set of conventions for static code & type exports, but the more I've experimented, the more trade-offs I run into. So for the sake of keeping this PR small (and not breaking a bunch of downstream imports), I'll branch that off into a separate PR & discussion thread.
Tasks
plugin.tsfile and export apluginfunction fromindex.ts, per recommended new platform conventionsindex.ts[ ] Add shims for each individual service, since there will be legacy/shim code in those service definitions as wellNot necessary yet as there aren't currently shim codes in top-level service definitions