Skip to content

Conversation

@TinaHeiligers
Copy link
Contributor

@TinaHeiligers TinaHeiligers commented Nov 15, 2019

Summary

Moves the feature_catalogue plugin (introduced in PR48818 ) into the home plugin.

Dev Docs

The ui/registries/feature_catalogue module has been deprecated for removal in 8.0

Plugins wishing to migrate may remove their usage of ui/registries/feature_catalogue and rely on either:

// For legacy plugins
import { npSetup } from 'ui/new_platform';
npSetup.plugins.home.featureCatalogue.register(/* same details here */);

// For new plugins: first add 'home` to the list of `optionalPlugins` 
// in your kibana.json file. Then access the plugin directly in `setup`:

class MyPlugin {
  setup(core, plugins) {
    if (plugins.home) {
      plugins.home.featureCatalogue.register(/* same details here. */);
    }
  }
}

Note that the old module supported providing a Angular DI function to receive Angular dependencies. This is no longer supported as we migrate away from Angular.

Checklist

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

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support

For maintainers

@TinaHeiligers TinaHeiligers added Feature:NP Migration Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.7.0 v8.0.0 labels Nov 15, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@TinaHeiligers
Copy link
Contributor Author

Looks like this PR is built on top of #50444, right? When the other one is merged, this one will become easier to review.
@flash1293 PR #50444 has been merged. There are fewer files with changes now and this PR should be easier to review. For consistency, I renames the server plugin from HomePlugin to HomeServerPlugin. LMK if that shouldn't be done in this PR.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Code changes LGTM, didn't pull down to test

Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

Spaces changes LGTM

@TinaHeiligers TinaHeiligers force-pushed the NP_kibana_plugin_home_feature_catalogue branch from f4548b2 to a485ba2 Compare November 19, 2019 17:15
@TinaHeiligers
Copy link
Contributor Author

@joshdover I need a LGTM from the platform team before I can merge this PR. All this PR does is move the feature_catalogue plugin code to the home plugin as a service in the public folder.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

Looks great! Just two small suggestions.

I think it might be clearer if the services directory had top-level directories for each service so the source files don't get mixed up. For example:

services/
  feature_catalogue/
    index.ts
    feature_catalogue_registry.ts
  other_service/
    index.ts
    other_services.ts
  index.ts

May want to change the server-side as well.

@TinaHeiligers
Copy link
Contributor Author

Looks great! Just two small suggestions.

I think it might be clearer if the services directory had top-level directories for each service so the source files don't get mixed up. For example:

services/
  feature_catalogue/
    index.ts
    feature_catalogue_registry.ts
  other_service/
    index.ts
    other_services.ts
  index.ts

May want to change the server-side as well.

I was thinking the same thing! I'll do that in this PR because the next task is to migrate the sample data and that's going to be a much larger effort.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@TinaHeiligers TinaHeiligers merged commit e46f977 into elastic:master Nov 19, 2019
TinaHeiligers added a commit to TinaHeiligers/kibana that referenced this pull request Nov 19, 2019
* Moves feature_catalogue services into the home plugin public folder.
TinaHeiligers added a commit that referenced this pull request Nov 20, 2019
* Moves feature_catalogue services into the home plugin public folder.
@TinaHeiligers TinaHeiligers deleted the NP_kibana_plugin_home_feature_catalogue branch November 20, 2019 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:NP Migration release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants