Enforce camelCase format for a plugin id by mshustov · Pull Request #53759 · elastic/kibana

mshustov · 2019-12-23T11:07:49Z

Summary

Closes #52190, #51226
Validates that pluginId set in camelCase.
configPath fallback to plugin id formated to snake_case automatically.
Adds requirements to the plugin manifest docs.

I added my concerns about the current implementation #52190 (comment)

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~~
~~[ ] Documentation was added for features that require explanation or tutorials~~
~~[ ] Unit or functional tests were updated or added to match the most common scenarios~~
~~[ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

Dev docs

When creating a new platform plugin, you need to make sure that pluginId declared in camelCase within kibana.json manifest file. It might not match pluginPath, which is recommended to be in snake_case format.

// ok
"pluginPath": ["foo"],
"id": "foo"
// ok
"pluginPath": "foo_bar",
"id": "fooBar"

elasticmachine · 2019-12-23T11:13:32Z

Pinging @elastic/kibana-platform (Team:Platform)

mshustov · 2020-01-13T13:02:03Z

@pgayvallet @joshdover updated the fallback mechanism to format pluginId to snake_case automatically, following the outcome of #52190
There are 2 plugins with id in camelCase that don't specify pluginPath: uiActions & advancedUiActions. Both of them don't use config, therefore they shouldn't be affected by the pluginPath auto-formatting.

pgayvallet · 2020-01-14T07:46:07Z

src/core/server/plugins/types.ts

-   * to "id".
+   * to "id" in snake_case format.


NIT: Maybe add an exemple myId => my_id

mshustov · 2020-01-16T08:41:44Z

@elasticmachine merge upstream

mshustov · 2020-01-17T12:08:28Z

@elasticmachine merge upstream

mshustov · 2020-01-18T07:54:25Z

@elasticmachine merge upstream

kibanamachine · 2020-01-18T09:11:50Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 69aec7b

History

💚 Build #21051 succeeded 72dfdc6
💚 Build #20765 succeeded a47d2aa
💚 Build #20569 succeeded eb08294
💚 Build #19727 succeeded 8b689f3
💚 Build #18308 succeeded 43f9744

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* add isCamelCase function * add a warning if id is not in camelCase * document pluginId expected in camelCase * regen docs * add a test for logging * update tests. warn can be called several times for different reasons * pluginPath falls back to plugin id in snake_case * update tests * update docs * add example with id & configPath different formats Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* add isCamelCase function * add a warning if id is not in camelCase * document pluginId expected in camelCase * regen docs * add a test for logging * update tests. warn can be called several times for different reasons * pluginPath falls back to plugin id in snake_case * update tests * update docs * add example with id & configPath different formats Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* upstream/master: (24 commits) Show error page when accessing unavailable app (elastic#54656) [ML] Improving job wizards with datafeed aggregations (elastic#55180) remove flaly assetion. a license presence tested anyway (elastic#55289) fix commonly used ranges uptime (elastic#54930) [SIEM] Use proper icons on Detections view (elastic#55215) Fix: invalid translation referenced (elastic#54901) [State Management] Remove AppState from edit_index_pattern page (elastic#54104) Implements `getStartServices` on server-side (elastic#55156) Move vis_vega_type/data_model tests to jest (elastic#55186) [SIEM] [Detection Engine] Update status on rule details page (elastic#55201) Fix KQL value suggestions for nested fields (elastic#54820) Enforce camelCase format for a plugin id (elastic#53759) [SIEM] Detection engine cleanup for rule details/creation/edit page (elastic#55069) Remove nested root from index pattern (elastic#54978) [Reporting/Migration] ReportingSetup, LegacySetup (elastic#54198) [SIEM] [Detection Engine] Fixes duplicate rule action (elastic#55252) [SIEM] Detections add alert & signal tab (elastic#55127) Management API - redirect on disabled app path (elastic#55136) [SIEM][Detection Engine] Fixes critical regression on the backend with immutable and tags update local (elastic#55177) ...

* master: (108 commits) [ML] Single Metric Viewer: Fix job check. (elastic#55191) Show error page when accessing unavailable app (elastic#54656) [ML] Improving job wizards with datafeed aggregations (elastic#55180) remove flaly assetion. a license presence tested anyway (elastic#55289) fix commonly used ranges uptime (elastic#54930) [SIEM] Use proper icons on Detections view (elastic#55215) Fix: invalid translation referenced (elastic#54901) [State Management] Remove AppState from edit_index_pattern page (elastic#54104) Implements `getStartServices` on server-side (elastic#55156) Move vis_vega_type/data_model tests to jest (elastic#55186) [SIEM] [Detection Engine] Update status on rule details page (elastic#55201) Fix KQL value suggestions for nested fields (elastic#54820) Enforce camelCase format for a plugin id (elastic#53759) [SIEM] Detection engine cleanup for rule details/creation/edit page (elastic#55069) Remove nested root from index pattern (elastic#54978) [Reporting/Migration] ReportingSetup, LegacySetup (elastic#54198) [SIEM] [Detection Engine] Fixes duplicate rule action (elastic#55252) [SIEM] Detections add alert & signal tab (elastic#55127) Management API - redirect on disabled app path (elastic#55136) [SIEM][Detection Engine] Fixes critical regression on the backend with immutable and tags ...

fbaligand · 2020-01-22T21:32:56Z

So it means that an existing community plugin named “enhanced-table” has to change its id?
And so every plugin user has all his existing visualizations broken?

VijayDoshi · 2020-01-23T00:25:27Z

The warnings on startup are numerous, why is enforcement of camelCase useful here? I could be missing context so am happy to learn.

pgayvallet · 2020-01-23T07:59:57Z

@fbaligand This is currently only showing a warning. Post 8.0 this will probably be a requirement.

@VijayDoshi this is a subtask of enforcing some code/name conventions on plugins. See #52190 for context and reasons of this change.

fbaligand · 2020-01-23T08:11:55Z

Thanks for answer. Nice to see that real requirement won’t happen before 8.0.

I’m quite confused about the exact required name convention: is it enhancedTable or enhanced_table ?

pgayvallet · 2020-01-23T08:26:59Z

Plugin name should be camelCase and would be enhancedTable. As explained in #52190 the main reason for this is that plugin id is part of the plugin's contract, as it's this value that is used to retrieve a dependency plugin by it's id.

Associated default configuration path (if not explicitly specified in the plugin's kibana.json) would be enhanced_table (This is to ensure consistency with the ES stack's configuration properties naming conventions which uses snake_case for config files props)

fbaligand · 2020-01-23T08:48:06Z

Ok, thanks.

mshustov added 4 commits December 23, 2019 13:00

add isCamelCase function

0fb874b

add a warning if id is not in camelCase

7bf999c

document pluginId expected in camelCase

023d9ab

regen docs

e793281

mshustov requested a review from a team as a code owner December 23, 2019 11:07

add a test for logging

9a0b77e

mshustov added Feature:New Platform release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.6.0 labels Dec 23, 2019

mshustov added 2 commits January 6, 2020 10:56

Merge branch 'master' into issue-52190-id-camelCase

da1a956

update tests. warn can be called several times for different reasons

4ad3277

pgayvallet approved these changes Jan 6, 2020

View reviewed changes

mshustov added 5 commits January 7, 2020 12:08

Merge branch 'master' into issue-52190-id-camelCase

43f9744

Merge branch 'master' into issue-52190-id-camelCase

503aed0

pluginPath falls back to plugin id in snake_case

7c8cf87

update tests

379d0d9

update docs

8b689f3

mshustov requested review from a team and pgayvallet January 13, 2020 13:02

pgayvallet approved these changes Jan 14, 2020

View reviewed changes

mshustov added v7.7.0 and removed v7.6.0 labels Jan 15, 2020

mshustov added 2 commits January 15, 2020 15:38

Merge branch 'master' into issue-52190-id-camelCase

9ba7788

add example with id & configPath different formats

eb08294

Merge branch 'master' into issue-52190-id-camelCase

a47d2aa

Merge branch 'master' into issue-52190-id-camelCase

72dfdc6

joshdover approved these changes Jan 17, 2020

View reviewed changes

Merge branch 'master' into issue-52190-id-camelCase

69aec7b

mshustov merged commit ec31611 into elastic:master Jan 18, 2020

mshustov deleted the issue-52190-id-camelCase branch January 18, 2020 13:17

mshustov mentioned this pull request Jan 18, 2020

[7.x] Enforce camelCase format for a plugin id (#53759) #55270

Merged

mshustov added v8.0.0 backported labels Jan 21, 2020

dblock mentioned this pull request Apr 28, 2021

Standardize plugin naming conventions opensearch-project/opensearch-plugins#10

Closed

Conversation

mshustov commented Dec 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Dev docs

Uh oh!

elasticmachine commented Dec 23, 2019

Uh oh!

mshustov commented Jan 13, 2020

Uh oh!

pgayvallet Jan 14, 2020

Choose a reason for hiding this comment

Uh oh!

mshustov commented Jan 16, 2020

Uh oh!

mshustov commented Jan 17, 2020

Uh oh!

mshustov commented Jan 18, 2020

Uh oh!

kibanamachine commented Jan 18, 2020

💚 Build Succeeded

History

Uh oh!

fbaligand commented Jan 22, 2020

Uh oh!

VijayDoshi commented Jan 23, 2020

Uh oh!

pgayvallet commented Jan 23, 2020

Uh oh!

fbaligand commented Jan 23, 2020

Uh oh!

pgayvallet commented Jan 23, 2020

Uh oh!

fbaligand commented Jan 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mshustov commented Dec 23, 2019 •

edited

Loading