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

[ML] Moves URL, interval and ordinal ui utils into ML #47221

Merged
merged 2 commits into from
Oct 3, 2019

Conversation

peteharverson
Copy link
Contributor

@peteharverson peteharverson commented Oct 3, 2019

Summary

As part of the migration to the new platform, copies several utility functions which were previously from the legacy ui folder into the ML codebase:

  • ui/url/relative_to_absolute (copied as-is into new ML util/url_utils)
  • ui/utils/ordinal_suffix (added new ML formatters/number_as_ordinal which uses the numeral.js ordinal format
  • ui/utils/parse_interval (switched all ML code to use the existing ML common/util/parse_interval

Also removed unused ui/kbn_top_nav import from app.js.

Note that the ML common/util/parse_interval differs from the legacy ui/utils/parse_interval in the following ways:

  1. A value-less interval such as 'm' is not allowed - in line with the ML back-end not accepting such interval Strings for the bucket span of a job.
  2. Zero length durations 0ms, 0s, 0m and 0h are accepted as-is. Note the job wizard implements a separate check that the bucket span is not zero length.
  3. Fractional intervals e.g. 1.5h or 4.5d are not allowed, in line with the behavior of the Elasticsearch date histogram aggregation.

Checklist

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@peteharverson peteharverson merged commit 8b561c7 into elastic:master Oct 3, 2019
@peteharverson peteharverson deleted the ml-ui-imports-move branch October 3, 2019 16:09
peteharverson added a commit to peteharverson/kibana that referenced this pull request Oct 3, 2019
* [ML] Moves URL, interval and ordinal ui utils into ML

* [ML] Edits to url_utils following review
peteharverson added a commit that referenced this pull request Oct 4, 2019
* [ML] Moves URL, interval and ordinal ui utils into ML

* [ML] Edits to url_utils following review
@peteharverson peteharverson mentioned this pull request Oct 30, 2019
78 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants