Skip to content

Conversation

@romseygeek
Copy link
Contributor

Type parameters are no longer used in DeleteRequest or UpdateRequest; this commit
removes them from the prepareX methods on Client, as well as removing setType()
and deprecated constructors on XRequestBuilder objects.

Relates to #41059

@romseygeek romseygeek added :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >refactoring v8.0.0 labels Oct 17, 2019
@romseygeek romseygeek self-assigned this Oct 17, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/CRUD)

@romseygeek
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/packaging-sample

@romseygeek
Copy link
Contributor Author

@elasticmachine update branch

@jtibshirani
Copy link
Contributor

@romseygeek to check I understand the change -- in addition to removing typed versions of prepareUpdate and prepareDelete, the PR removes prepareIndex(index, type). But prepareIndex(index, type, id) is still present and will be removed in a future PR?

@romseygeek
Copy link
Contributor Author

I had intended to remove prepareIndex(index, type) in a separate PR but rebased against the wrong branch apparently. I can separate the two changes out if that makes it easier to review?

@romseygeek
Copy link
Contributor Author

@elasticmachine update branch

@romseygeek
Copy link
Contributor Author

@elasticmachine update branch

@jtibshirani
Copy link
Contributor

I had intended to remove prepareIndex(index, type) in a separate PR but rebased against the wrong branch apparently. I can separate the two changes out if that makes it easier to review?

If it's not a ton of work, I think that would be cleanest. But if it will take significant time, we could just update the PR description to clearly list out what has changed, and mention the plan for a follow-up PR?

@jtibshirani
Copy link
Contributor

@romseygeek and I discussed offline -- we will keep the change as-is because would be a large amount of work to separate it out.

Copy link
Contributor

@jtibshirani jtibshirani 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 good to me.

@romseygeek
Copy link
Contributor Author

@elasticmachine update branch

@romseygeek romseygeek merged commit e85e3a8 into elastic:master Oct 23, 2019
@romseygeek romseygeek deleted the types-removal/prepare-index branch October 23, 2019 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >refactoring v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants