-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[data] Extract @kbn/field-types to a package #106973
Conversation
…-package # Conflicts: # docs/development/plugins/data/public/kibana-plugin-plugins-data-public.md # docs/development/plugins/data/server/kibana-plugin-plugins-data-server.md
@elasticmachine merge upstream |
Pinging @elastic/kibana-app-services (Team:AppServices) |
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.
Didn't checkout, but it overall looks awesome.
Added a couple of comments (texts and review for the bazel file from the ops team).
Other than that LGTM
@@ -0,0 +1,89 @@ | |||
|
|||
load("@npm//@bazel/typescript:index.bzl", "ts_config", "ts_project") |
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.
Think it could be a good idea to have someone from @elastic/kibana-operations review this. What do you say?
} from '@kbn/field-types'; | ||
|
||
/** | ||
* @deprecated import from "@kbn/field-types" instead |
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.
I think it could be nice if we use identical messages across Kibana.
Maybe align with the format I started (or change to yours if you prefer).
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.
I agree, I made this message a bit nicer and add @removeBy
tag.
We need to add a @removeBy
tag and change kbn/es-query
-> @kbn/es-query
in other messages, but I don't want to do this in this pr. I would prefer a separate pr instead. There we can also align those.
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.
I made #107175
Mind reviewing?
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.
just a couple minor comments, otherwise LGTM
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.
I tested on Windows - LGTM
@elasticmachine merge upstream |
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/dashboard_mode/dashboard_view_mode·js.dashboard mode Dashboard View Mode Dashboard viewer "before all" hook: Create dashboard only mode user for "shows only the dashboard app link"Standard Out
Stack Trace
Kibana Pipeline / general / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/security/users·js.security app users should show the default rolesStandard Out
Stack Trace
Kibana Pipeline / general / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/security/users·js.security app users should show the default rolesStandard Out
Stack Trace
Metrics [docs]Module Count
Public APIs missing comments
Page load bundle
Unknown metric groupsReferences to deprecated APIs
History
To update your PR or re-run it, just comment with: |
Summary
Needed for #106966
This extracts leaf
kbn_field_types
utils and types to a package so that we can break out field formats and index patterns from data.I re-exported the package from data plugin and marked old exports as deprecated where it doesn't hurt. I converted only imports inside
field formats
code so we have fewer changes in #106966Checklist
Plugin API changes
kbn_field_types
were extracted fromdata
plugin into a separate package@kbn/field-types