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.

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

[New Platform Migration Phase I]: update dateHisogramInterval & parseEsInterval imports #42917

Merged
merged 7 commits into from
Aug 13, 2019

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Aug 8, 2019

Partially fix: #40553

Summary

In #39091, dateHistogramInterval, parseEsInterval and a few others were moved to the data plugin. However, imports from ui/public/utils were not switched over.

We need to update imports across Kibana so that we can remove ui/public/utils/parse_es_interval completely and consider Phase I "done" for this item.

Checklist

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

For maintainers

@alexwizp alexwizp requested review from timroes, lukeelmers, lizozom and a team August 8, 2019 10:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@elasticmachine
Copy link
Contributor

💔 Build Failed

@@ -26,7 +26,7 @@ import {

import {
parseEsInterval,
} from 'ui/utils/parse_es_interval';
} from '../../../../../../../../../src/legacy/core_plugins/data/public';
Copy link
Contributor

Choose a reason for hiding this comment

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

:-(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

=( but we should use relative imports now

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I know. Was just acknowledging that it's ugly.

parseEsInterval,
ParsedInterval,
} from '../common/parse_es_interval';
export * from '../common';
Copy link
Contributor

Choose a reason for hiding this comment

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

<3

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom
Copy link
Contributor

lizozom commented Aug 8, 2019

For the type of errors in your jest tests, I found this style of mocking very useful:

jest.mock('../../../../../data/public', () => {
  return {
    FilterBar: () => <div className="filterBar"></div>,
    ... any other components and services you actually need...
  };
});

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alexwizp alexwizp requested a review from lizozom August 9, 2019 05:45
@alexwizp alexwizp requested a review from a team as a code owner August 9, 2019 10:36
@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.

Changes to Core look fine 👍

Copy link
Contributor

@rudolf rudolf left a comment

Choose a reason for hiding this comment

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

Just a small error in the docs

src/core/MIGRATION.md Outdated Show resolved Hide resolved
Co-Authored-By: Rudolf Meijering <[email protected]>
@alexwizp alexwizp requested a review from rudolf August 9, 2019 19:46
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lizozom
Copy link
Contributor

lizozom commented Aug 11, 2019

Code LGTM.

Didn't test because I'm unfamiliar with rollups (where these are mostly used).
@alexwizp lets chat.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

Tested in rollups and LGTM

@alexwizp alexwizp merged commit a5284a5 into elastic:master Aug 13, 2019
alexwizp added a commit to alexwizp/kibana that referenced this pull request Aug 13, 2019
…EsInterval imports (elastic#42917)

* [New Platform Migration Phase I]: update dateHisogramInterval & parseEsInterval imports

* fix tests

* modify MIGRATION.md

* Update src/core/MIGRATION.md

Co-Authored-By: Rudolf Meijering <[email protected]>

# Conflicts:
#	src/core/MIGRATION.md
alexwizp added a commit that referenced this pull request Aug 13, 2019
…EsInterval imports (#42917) (#43169)

* [New Platform Migration Phase I]: update dateHisogramInterval & parseEsInterval imports

* fix tests

* modify MIGRATION.md

* Update src/core/MIGRATION.md

Co-Authored-By: Rudolf Meijering <[email protected]>

# Conflicts:
#	src/core/MIGRATION.md
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 13, 2019
…p-metrics-selectall

* 'master' of github.com:elastic/kibana: (27 commits)
  [ML] Data Frames: Analytics job creation. (elastic#43102)
  [Vis Default editor] Fix issue with Rollup (elastic#42430)
  [Vis: Default editor] EUIficate Markdown tab (elastic#42677)
  [New Platform Migration Phase I]: update dateHisogramInterval & parseEsInterval imports (elastic#42917)
  [Infra UI] Add AWS metrics to node detail page (elastic#42153)
  update apm index pattern (elastic#43106)
  [SIEM] Toggle Column / Code Coverage and Cypress (elastic#42766)
  skip failing test (elastic#43163)
  [code] Add option to turn the go dependency download on/off. (elastic#43096)
  disable visual regression jobs
  Removed dead code (elastic#42774)
  fixes csv export of saved searches that have _source field (elastic#43123)
  Export missing Context types (elastic#43051)
  Update dependency supports-color to v7 (elastic#43064)
  switch to icon type string instead of node (elastic#43111)
  [Maps] Enable borders for icon symbols (elastic#43066)
  [ftr] enable visualRegression jobs (elastic#42989)
  [ML] Converting single to multi metric job (elastic#42532)
  fix(NA): dont clean dll module if it is a package json file (elastic#42904)
  [Logs UI] Add link from the sample web logs to the Logs UI (elastic#42635)
  ...
@alexwizp alexwizp deleted the parse_es_interval_imports branch January 4, 2020 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Platform Migration Phase I]: update dateHisogramInterval & parseEsInterval imports
6 participants