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

Remove unused generic type for client execute method #31444

Merged
merged 5 commits into from
Jun 20, 2018

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Jun 19, 2018

This commit removes the request builder generic type for AbstractClient
as it was unused.

This commit removes the request builder generic type for AbstractClient
as it was unused.
@rjernst rjernst added :Core/Infra/Core Core issues without another label v7.0.0 >refactoring labels Jun 19, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@rjernst rjernst requested a review from imotov June 19, 2018 21:21
Copy link
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

Left a really minor comment about formatting. It would be great if we could clean up the format a bit while here. Otherwise LGTM.

RequestBuilder extends ActionRequestBuilder<Request, Response>> void doExecute(
Action<Response> action, Request request, ActionListener<Response> listener) {
protected <Request extends ActionRequest, Response extends ActionResponse>
void doExecute(Action<Response> action, Request request, ActionListener<Response> listener) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This formatting looks a bit weird to me, maybe switch to the same format as the next file instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually prefer this formatting to the one in FilterClient, as it maintains the return type in an easily viewable place. The FilterClient example also at first glance looks like the parameters are local variables because of indentation, until you notice the curly at the end of the line. Given that reasoning, would you be happy if I still made all these cases the same formatting?

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 see your point. Let's just make it more consistent then.

@rjernst rjernst merged commit 00283a6 into elastic:master Jun 20, 2018
@rjernst rjernst deleted the deguice37 branch June 20, 2018 23:26
@rjernst
Copy link
Member Author

rjernst commented Jun 20, 2018

Thanks for the review @imotov!

dnhatn added a commit that referenced this pull request Jun 23, 2018
* master:
  Add get field mappings to High Level REST API Client (#31423)
  [DOCS] Updates Watcher examples for code testing (#31152)
  TEST: Add bwc recovery tests with synced-flush index
  [DOCS] Move sql to docs (#31474)
  [DOCS] Move monitoring to docs folder (#31477)
  Core: Combine doExecute methods in TransportAction (#31517)
  IndexShard should not return null stats (#31528)
  fix repository update with the same settings but different type (#31458)
  Fix Mockito trying to mock IOException that isn't thrown by method (#31433) (#31527)
  Node selector per client rather than per request (#31471)
  Core: Combine messageRecieved methods in TransportRequestHandler (#31519)
  Upgrade to Lucene 7.4.0. (#31529)
  [ML] Add ML filter update API (#31437)
  Allow multiple unicast host providers (#31509)
  Avoid deprecation warning when running the ML datafeed extractor. (#31463)
  REST high-level client: add simulate pipeline API (#31158)
  Get Mapping API to honour allow_no_indices and ignore_unavailable (#31507)
  [PkiRealm] Invalidate cache on role mappings change (#31510)
  [Security] Check auth scheme case insensitively (#31490)
  In NumberFieldType equals and hashCode, make sure that NumberType is taken into account. (#31514)
  [DOCS] Fix REST tests in SQL docs
  [DOCS] Add code snippet testing in more ML APIs (#31339)
  Core: Remove ThreadPool from base TransportAction (#31492)
  [DOCS] Remove fixed file from build.gradle
  Rename createNewTranslog to fileBasedRecovery (#31508)
  Test: Skip assertion on windows
  [DOCS] Creates field and document level security overview (#30937)
  [DOCS] Significantly improve SQL docs
  [DOCS] Move migration APIs to docs (#31473)
  Core: Convert TransportAction.execute uses to client calls (#31487)
  Return transport addresses from UnicastHostsProvider (#31426)
  Ensure local addresses aren't null (#31440)
  Remove unused generic type for client execute method (#31444)
  Introduce http and tcp server channels (#31446)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >refactoring v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants