[TSVB] Renamed 'positive rate' to 'counter rate' by batchunag · Pull Request #80939 · elastic/kibana

batchunag · 2020-10-18T02:44:31Z

Summary

Renamed the label and corresponding function/file names.
Todo: i18n translations for label.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

kibanamachine · 2020-10-18T02:44:34Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

cla-checker-service · 2020-10-18T02:44:36Z

💚 CLA has been signed

batchunag · 2020-10-18T02:50:29Z

Closes #80228

timroes · 2020-10-19T07:48:58Z

Hi,

thanks a lot for this PR. Could you please make sure to sign the CLA above for us to be able to merge this code.

A couple of notes:

I think we should not rename the "id" of the aggregation as done in this PR. Otherwise you'll also need to provide a saved object migration, that will migrate previously created saved objects from the old id to the new one. I would just leave it as it is and just change the labels and therefore not requiring a saved object migration.
For translation of those labels (since I saw you left a TODO in your description): You only need to remove the labels you changed from all translation files, (e.g. remove the visTypeTimeseries.aggLookup.positiveRateLabel and other changed ids from the ja-JP.json and zh-CN.json file under x-pack/plugins/translations/translations). Our translation team will then take care of translating them for the new release again.

Cheers,
Tim

elasticmachine · 2020-10-29T10:30:36Z

Pinging @elastic/kibana-app (Team:KibanaApp)

batchunag · 2020-10-29T10:31:11Z

@elasticmachine merge upstream

batchunag · 2020-10-29T10:34:20Z

@timroes thanks for the feedback. I've updated the code.
Regarding to the CLA, I've already signed. Do I have to submit the pdf file to somewhere? I couldn't find information on that.

timroes · 2020-10-29T10:40:28Z

@batchunag I see that you signed the CLA with a @gmail.com mail address, but you made this PR from a @indeed.com mail address. Could you please either change the author of the commits, or sign the CLA also for your @indeed.com mail address? Thank you

batchunag · 2020-10-29T10:42:30Z

Oh I see. Thanks.

batchunag · 2020-10-29T10:51:30Z

@elasticmachine merge upstream

batchunag · 2020-10-29T10:52:15Z

@timroes CLA works now.

stratoula · 2020-10-29T11:32:52Z

@batchunag thank you a lot for this PR. I haven't made a thorough code review but I have two comments so far:

As Tim has already mentioned we would like to only change the labels of the positive rate to counter rate and not the type. Now when I try to select the Counter rate it is not selected and a wrong message is displayed:

that the `positive_rate` aggregation is not supported which is not the case here 😄

You have removed the translations from x-pack/plugins/translations/translations/ja-JP.json but you should also do it forx-pack/plugins/translations/translations/zh-CN.json

batchunag · 2020-10-29T14:05:53Z

@stratoula thanks for the feedback. Looks like I'm making the code less readable. I was hesitant to leave some variable, method names half changed. Okay, I'll keep my changes only to the label text part.

batchunag · 2020-10-29T15:13:13Z

@stratoula now we've much simpler update. Please have a look. I think the other labels such as visTypeTimeseries.positiveRate.helpText: "This aggregation should only be applied to {link}, it is a shortcut for applying max, derivative and counter only to a field." still valid regardless of the labels changed.

stratoula · 2020-10-29T15:33:48Z

@elasticmachine merge upstream

stratoula · 2020-10-29T15:37:06Z

Jenkins, test this.

stratoula · 2020-10-29T15:40:16Z

@stratoula now we've much simpler update. Please have a look. I think the other labels such as visTypeTimeseries.positiveRate.helpText: "This aggregation should only be applied to {link}, it is a shortcut for applying max, derivative and counter only to a field." still valid regardless of the labels changed.

Thank you @batchunag! Yes, it is fine. Will check it asap 🙂

kibanamachine · 2020-10-29T17:15:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 703cc85

Metrics [docs]

async chunks size

id	before	after	diff
`visTypeTimeseries`	1.7MB	1.7MB	-3.0B

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

stratoula

Code LGTM, tested it locally in Chrome and Safari. The positive rate has been renamed to counter rate everywhere. Thanx @batchunag for working on this 🍪

* Renamed 'positive rate' label text to 'counter rate' * Removed translations(ja-JP, zh-CN) where the labels were updated Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* Renamed 'positive rate' label text to 'counter rate' * Removed translations(ja-JP, zh-CN) where the labels were updated Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: DB <batchunag@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* master: (71 commits) [Chrome] Extension to append an element to the last breadcrumb (elastic#82015) [Monitoring] Thread pool rejections alert (elastic#79433) [Actions] Fix actionType type on registerType function (elastic#82125) [Security Solution] Modal for saving timeline (elastic#81802) add tests for index pattern switching (elastic#81987) TS project references for share plugin (elastic#82051) [Graph] Fix problem with duplicate ids (elastic#82109) skip 'returns a single bucket if array has 1'. related elastic#81460 Add a link to documentation in the alerts and actions management UI (elastic#81909) [Fleet] fix duplicate ingest pipeline refs (elastic#82078) Context menu trigger for URL Drilldown (elastic#81158) SO management: fix legacy import index pattern selection being reset when switching page (elastic#81621) Fixed dead links (elastic#78696) [Search] Add "restore" to session service (elastic#81924) fix Lens heading structure (elastic#81752) [ML] Data Frame Analytics: Fix feature importance cell value and decision path chart (elastic#82011) Remove legacy app arch items from codeowners. (elastic#82084) [TSVB] Renamed 'positive rate' to 'counter rate' (elastic#80939) Expressions/migrations2 (elastic#81281) [Telemetry] [Schema] remove number type and support all es number types (elastic#81774) ...

batchunag requested a review from a team October 18, 2020 02:44

kibanamachine added the 💝community label Oct 18, 2020

batchunag marked this pull request as draft October 18, 2020 02:45

batchunag force-pushed the master branch from 9f01967 to c71a887 Compare October 18, 2020 11:27

timroes added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Oct 19, 2020

batchunag marked this pull request as ready for review October 29, 2020 10:30

batchunag force-pushed the master branch 2 times, most recently from a747cd8 to 53bf7a6 Compare October 29, 2020 10:51

timroes assigned stratoula Oct 29, 2020

batchunag added 2 commits October 30, 2020 00:09

Renamed 'positive rate' label text to 'counter rate'

564d4e4

Removed translations(ja-JP, zh-CN) where the labels were updated

27fb0f2

batchunag force-pushed the master branch from c0aa22e to 27fb0f2 Compare October 29, 2020 15:10

stratoula added Feature:TSVB TSVB (Time Series Visual Builder) release_note:skip Skip the PR/issue when compiling release notes v7.11 v8.0.0 labels Oct 29, 2020

Merge branch 'master' into master

703cc85

wylieconlon added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Oct 29, 2020

wylieconlon changed the title ~~Renamed 'positive rate' to 'counter rate'~~ [TSVB] Renamed 'positive rate' to 'counter rate' Oct 29, 2020

stratoula approved these changes Oct 30, 2020

View reviewed changes

stratoula merged commit 534f4e8 into elastic:master Oct 30, 2020

stratoula mentioned this pull request Oct 30, 2020

[7.x] [TSVB] Renamed 'positive rate' to 'counter rate' (#80939) #82102

Merged

timroes added v7.11.0 and removed v7.11 labels Oct 30, 2020

timroes mentioned this pull request Nov 2, 2020

[TSVB] Rename positive rate to counter rate #80228

Closed

Conversation

batchunag commented Oct 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

kibanamachine commented Oct 18, 2020

Uh oh!

cla-checker-service bot commented Oct 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

batchunag commented Oct 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

timroes commented Oct 19, 2020

Uh oh!

elasticmachine commented Oct 29, 2020

Uh oh!

batchunag commented Oct 29, 2020

Uh oh!

batchunag commented Oct 29, 2020

Uh oh!

timroes commented Oct 29, 2020

Uh oh!

batchunag commented Oct 29, 2020

Uh oh!

batchunag commented Oct 29, 2020

Uh oh!

batchunag commented Oct 29, 2020

Uh oh!

stratoula commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

batchunag commented Oct 29, 2020

Uh oh!

batchunag commented Oct 29, 2020

Uh oh!

stratoula commented Oct 29, 2020

Uh oh!

stratoula commented Oct 29, 2020

Uh oh!

stratoula commented Oct 29, 2020

Uh oh!

kibanamachine commented Oct 29, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

batchunag commented Oct 18, 2020 •

edited

Loading

cla-checker-service bot commented Oct 18, 2020 •

edited

Loading

batchunag commented Oct 18, 2020 •

edited

Loading

stratoula commented Oct 29, 2020 •

edited

Loading