Skip to content

Dry up TransportResponseHandler #64395

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

Conversation

original-brownbear
Copy link
Contributor

I keep typing this over and over so I figured I'd dry it up:

The majority of implementations of this interface run on SAME.
Drying this up saves some duplication and also makes it a little
easier to understand what callbacks won't fork-off when reading the
code.

The majority of implementations of this interface run on `SAME`.
Drying this up saves some duplication and also makes it a little
easier to understand what callbacks won't fork-off when reading the
code.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Core)

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Oct 30, 2020
@original-brownbear
Copy link
Contributor Author

Jenkins run elasticsearch-ci/bwc
Jenkins run elasticsearch-ci/default-distro

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

This looks fine, but is there any reason this couldn't live as a default implementation of executor() on the TransportResponseHandler interface?

@original-brownbear
Copy link
Contributor Author

This looks fine, but is there any reason this couldn't live as a default implementation of executor() on the TransportResponseHandler interface?

yea that's the cleaner approach indeed -> moved to that now thanks! :)

@original-brownbear
Copy link
Contributor Author

Jenkins run elasticsearch-ci/2

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@original-brownbear
Copy link
Contributor Author

Thanks Ryan!

@original-brownbear original-brownbear merged commit 13f1d1a into elastic:master Nov 5, 2020
@original-brownbear original-brownbear deleted the shorten-transport-response-handlers branch November 5, 2020 06:39
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Nov 5, 2020
The majority of implementations of this interface run on `SAME`.
Drying this up saves some duplication and also makes it a little
easier to understand what callbacks won't fork-off when reading the
code.
original-brownbear added a commit that referenced this pull request Nov 5, 2020
The majority of implementations of this interface run on `SAME`.
Drying this up saves some duplication and also makes it a little
easier to understand what callbacks won't fork-off when reading the
code.
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 >non-issue Team:Core/Infra Meta label for core/infra team v7.11.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants