-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Index Patterns] Move rollup config to index pattern management #102145
[Index Patterns] Move rollup config to index pattern management #102145
Conversation
Pinging @elastic/kibana-app-services (Team:AppServices) |
@@ -8,3 +8,5 @@ | |||
|
|||
export { IndexPatternCreationConfig, IndexPatternCreationOption } from './config'; | |||
export { IndexPatternCreationManager } from './manager'; | |||
// @ts-ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rollups had two js
files that I'd prefer not to convert to typescript at this point. There are plans for rollup improvements that would make these files unnecessary.
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is duplicated from the rollup plugin. I thought it would be better than creating a dependency but open to discuss.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM! I think that removing this indirection and the dependency of rollups upon index pattern management makes the code easier to understand. I haven't tested locally so I'm assuming you verified that rollup index patterns can still be created, edited, and listed.
💚 Build SucceededMetrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
References to deprecated APIs
History
To update your PR or re-run it, just comment with: |
…tic#102145) * move rollup config to index pattern management
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…tic#102145) * move rollup config to index pattern management
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
Summary
TLDR; Removing
rollup
plugin's dependence on theindexPatternManagment
plugin will allow the (in progress)indexPatternEditor
plugin to determine if therollup
plugin is active, otherwise there's a circular dependency. This is also tech debt reduction.Refactor index pattern creation code so its all within the index pattern management plugin. Previously rollup related code was in the rollup plugin since it was an x-pack feature. We're no longer creating one off APIs to maintain the distinction between x-pack features and OSS code.