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

[data] Clean up QueryStringInput unit tests #58704

Merged
merged 2 commits into from
Mar 2, 2020

Conversation

DianaDerevyankina
Copy link
Contributor

@DianaDerevyankina DianaDerevyankina commented Feb 27, 2020

Closes: #58165

Summary

Got rid of snapshot generation and refactored localization in components, so i18n is used now instead of InjectedIntl and injectI18n.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cla-checker-service
Copy link

cla-checker-service bot commented Feb 27, 2020

💚 CLA has been signed

@DianaDerevyankina
Copy link
Contributor Author

cla/check

@DianaDerevyankina DianaDerevyankina self-assigned this Feb 27, 2020
@DianaDerevyankina DianaDerevyankina added v8.0.0 v7.7.0 release_note:skip Skip the PR/issue when compiling release notes Team:AppArch labels Feb 27, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@DianaDerevyankina DianaDerevyankina added the Feature:Query Bar Querying and query bar features label Feb 27, 2020
@alexwizp
Copy link
Contributor

cla/check

Copy link
Contributor

@alexwizp alexwizp left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you

@alexwizp alexwizp marked this pull request as ready for review February 27, 2020 12:28
@alexwizp alexwizp requested a review from a team as a code owner February 27, 2020 12:28
@@ -584,4 +582,4 @@ export class QueryStringInputUI extends Component<Props, State> {
}
}

export const QueryStringInput = injectI18n(withKibana(QueryStringInputUI));
export const QueryStringInput = withKibana(QueryStringInputUI);
Copy link
Contributor

Choose a reason for hiding this comment

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

We can get rid of the variable and rename the class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

QueryStringInput and QueryStringInputUI are both used in the other code pieces, so we can't get rid of any of them

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could replace the existing usages, but its not critical

@alexwizp
Copy link
Contributor

cla/check

1 similar comment
@alexwizp
Copy link
Contributor

cla/check

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.

LGTM

@maryia-lapata
Copy link
Contributor

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@DianaDerevyankina DianaDerevyankina merged commit 37d18a7 into elastic:master Mar 2, 2020
@DianaDerevyankina DianaDerevyankina deleted the 58165 branch March 2, 2020 09:22
jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 2, 2020
…dex-server-side

* 'master' of github.com:elastic/kibana: (34 commits)
  [Upgrade Assistant] Remove "boom" from reindex service (elastic#58715)
  [data] Clean up QueryStringInput unit tests (elastic#58704)
  [SIEM] Detection Fix typo in Adobe Hijack Persistence rule (elastic#58804)
  Restores [SIEM][CASE] Init Configure Case Page (elastic#58121) (elastic#58924)
  Skips additional failing Ingest Manager integration tests
  Skips failing Ingest Manager integration tests
  Move dev tools styles to NP (elastic#58855)
  change to have kibana --ssl cli option use more recent certs (elastic#57933)
  disable failing suite (elastic#58942)
  Don't start pollEsNodesVersion unless someone subscribes (elastic#56923)
  Do not write UUID file during optimize process (elastic#58899)
  [Endpoint] Task/add nav bar (elastic#58604)
  [Metric Alerts] Add backend support for multiple expressions per alert  (elastic#58672)
  [Metrics Alerts] Fix alerting on a rate aggregation (elastic#58789)
  disable flaky suite (elastic#55953)
  Revert "[SIEM] apollo@3 (elastic#51926)" and "[SIEM][CASE] Init Confi… (elastic#58806)
  [resubmit] Prep agg types for new platform (elastic#58893)
  [Lens] Allow number formatting within Lens (elastic#56253)
  [Autocomplete] Use settings from config rather than UI settings (elastic#58784)
  Improve action and trigger types (elastic#58657)
  ...

# Conflicts:
#	x-pack/plugins/upgrade_assistant/server/routes/reindex_indices/reindex_indices.ts
DianaDerevyankina added a commit to DianaDerevyankina/kibana that referenced this pull request Mar 2, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 3, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 3, 2020
DianaDerevyankina added a commit that referenced this pull request Mar 3, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration Feature:Query Bar Querying and query bar features release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data] Clean up QueryStringInput unit tests
6 participants