Skip to content

[new-platform] Improve naming and consistency in Plugin types - #34725

Merged
joshdover merged 5 commits into
elastic:masterfrom
joshdover:cleanup-dependency-naming
Apr 10, 2019
Merged

joshdover merged 5 commits into
elastic:masterfrom
joshdover:cleanup-dependency-naming

Conversation

@joshdover

@joshdover joshdover commented Apr 8, 2019 •

Copy link
Copy Markdown
Contributor

Summary

This improves the naming and consistency of the types for the PluginServices between the public and server versions.

@joshdover joshdover added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// Feature:New Platform v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.2.0 labels Apr 8, 2019
@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-platform

@joshdover
joshdover requested a review from a team as a code owner April 8, 2019 16:04
@joshdover
joshdover force-pushed the cleanup-dependency-naming branch from 1c68c59 to a8b8d3f Compare April 8, 2019 16:06
@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@eliperelman eliperelman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great.

Comment thread src/core/server/plugins/plugins_system.ts Outdated
@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Comment thread src/core/public/plugins/plugin.ts Outdated
@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@rudolf rudolf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The last two dependency parameters aren't as critical as the public API, but would be nice to have these consistent too.

Comment thread src/core/public/plugins/plugin.ts Outdated
* is the contract returned by the dependency's `setup` function.
*/
public async setup(setupContext: PluginSetupContext, dependencies: TDependenciesSetup) {
public async setup(setupContext: PluginSetupContext, dependencies: TPluginsSetup) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not as critical as the public API, but probably worth making this dependency paramater consistent too

Comment thread src/core/server/plugins/plugin.ts Outdated
* is the contract returned by the dependency's `setup` function.
*/
public async setup(setupContext: PluginSetupContext, dependencies: TDependenciesSetup) {
public async setup(setupContext: PluginSetupContext, dependencies: TPluginsSetup) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

another dependency parameter

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
…c#34725)

* Improve consistency in Plugin types

* Use #has() instead of undefined check

* Rename parameter

* Update core docs

* Internal updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.2.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants