Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move ui/field_formats to src/plugins/data #44883

Merged
merged 1 commit into from
Sep 16, 2019
Merged

Move ui/field_formats to src/plugins/data #44883

merged 1 commit into from
Sep 16, 2019

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Sep 5, 2019

Summary

Part of #43439 Moving src/legacy/ui/field_formats/ to src/plugins/data

Checklist

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

For maintainers

@alexwizp alexwizp requested a review from a team as a code owner September 5, 2019 13:32
@streamich streamich mentioned this pull request Sep 5, 2019
Closed
7 tasks
alexwizp
alexwizp commented Sep 5, 2019
View reviewed changes
src/plugins/data/common/field_formats/utils/as_pretty_string.ts Outdated
*/

// @ts-ignore
export * from '../../../../../legacy/core_plugins/kibana/common/utils/as_pretty_string';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

temporary reference. as_pretty_string will be moved to this file in a sepate PR.

src/plugins/data/common/field_formats/utils/highlight_html.ts Outdated
*/

// @ts-ignore
export { getHighlightHtml } from '../../../../../legacy/core_plugins/kibana/common/highlight';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

temporary reference. getHighlightHtml will be moved to this file in a sepate PR.

@alexwizp alexwizp self-assigned this Sep 5, 2019
@alexwizp alexwizp requested a review from a team as a code owner September 5, 2019 15:36
@elastic elastic deleted a comment from elasticmachine Sep 5, 2019
@lizozom
Copy link
Contributor

lizozom commented Sep 5, 2019

Not sure if this is a timely question, but why are we putting this into data?
And if it's moving to data, why not into index patterns?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alexwizp alexwizp force-pushed the field_formats_moving branch from 37bf735 to a895070 Compare September 6, 2019 07:18
@alexwizp
Copy link
Contributor Author

alexwizp commented Sep 6, 2019

@lukeelmers @ppisljar

Not sure if this is a timely question, but why are we putting this into data?
And if it's moving to data, why not into index patterns?

Could you please comment it.

@alexwizp alexwizp force-pushed the field_formats_moving branch from a895070 to 3247ddb Compare September 6, 2019 08:01
@alexwizp alexwizp mentioned this pull request Sep 6, 2019
Closed
3 tasks
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alexwizp
Copy link
Contributor Author

alexwizp commented Sep 6, 2019

@elastic/ml-ui can you please take a look?

peteharverson
peteharverson approved these changes Sep 6, 2019
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

ML edit LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ppisljar
Copy link
Member

ppisljar commented Sep 9, 2019

lets wait with this (Alex is on vacation) we need to spend some more time thinking about what exactly to do with field_formats

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@gospodarsky
Copy link

@ppisljar While Alex is on vacation I'm able to lead the PR and take a part of thinking if you've some proposals.

@gospodarsky
Copy link

@lukeelmers @ppisljar

Not sure if this is a timely question, but why are we putting this into data?
And if it's moving to data, why not into index patterns?

Could you please comment it.

@lukeelmers @ppisljar ^^

@ppisljar
Copy link
Member

ppisljar commented Sep 11, 2019 •
edited
Loading

we are moving it into data plugin, as at the moment it seems to be only consumed from there.
we are not putting it into index patterns as its used by aggconfigs as well.

as part of the move we should also: (not all need to be in this PR, we should rather break it down)

  • remove ui/registries/field_formats and replace it with registerFormat/getFormat/getFormats functions on the field_formats service inside data plugin
  • move ui/field_wildcard to field_formats service ?
  • move server side registration of field formats from kibana plugin to field_formats service
  • decouple agg configs from field_formats .... rather than importing the field formats agg_config should list prefered format which can then be looked up in the registry at render times.
  • decide what to do with UI for field_formats (ui/field_editor and ui/registry/field_format_editors)
    • have it with field formats service
    • have it with management app

@alexwizp alexwizp force-pushed the field_formats_moving branch 2 times, most recently from 2caa39e to dda9f0b Compare September 16, 2019 10:26
@elastic elastic deleted a comment from elasticmachine Sep 16, 2019
@alexwizp alexwizp force-pushed the field_formats_moving branch from dda9f0b to a582d36 Compare September 16, 2019 11:32
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

ppisljar
ppisljar approved these changes Sep 16, 2019
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@alexwizp alexwizp merged commit 16cab29 into elastic:master Sep 16, 2019
@alexwizp alexwizp mentioned this pull request Sep 16, 2019
Merged
alexwizp added a commit to alexwizp/kibana that referenced this pull request Sep 16, 2019
@alexwizp
Move ui/field_formats to src/plugins/data (elastic#44883)
60951af
alexwizp added a commit that referenced this pull request Sep 16, 2019
@alexwizp
Move ui/field_formats to src/plugins/data (#44883) (#45780)
d8b95c5
@alexwizp alexwizp deleted the field_formats_moving branch January 4, 2020 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppisljar ppisljar ppisljar approved these changes

@peteharverson peteharverson peteharverson approved these changes

@lizozom lizozom Awaiting requested review from lizozom

@lukeelmers lukeelmers Awaiting requested review from lukeelmers

Assignees

@alexwizp alexwizp

Labels
Feature:FieldFormatters Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes v7.5.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@alexwizp @lizozom @elasticmachine @ppisljar @gospodarsky @peteharverson