Skip to content

Conversation

@jakelandis
Copy link
Contributor

part of #34884

No changes beyond adding newlines to satisfy the 140 char line length.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jakelandis
Copy link
Contributor Author

tests now included. should be ready for review.

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM assuming CI is happy :)

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Left one thing, otherwise LGTM.

*/
public class DeleteRequest extends ReplicatedWriteRequest<DeleteRequest> implements DocWriteRequest<DeleteRequest>, CompositeIndicesRequest {
public class DeleteRequest extends ReplicatedWriteRequest<DeleteRequest> implements DocWriteRequest<DeleteRequest>,
CompositeIndicesRequest {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add the new line before implements instead of here? Could you also indent this one more time so it doesn't line up with the class body?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @nik9000, looks much better as suggested. (fixed)

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

❤️

@jakelandis jakelandis merged commit 11fa8d3 into elastic:master Oct 26, 2018
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Oct 29, 2018
kcm pushed a commit that referenced this pull request Oct 30, 2018
jakelandis added a commit that referenced this pull request Oct 30, 2018
@jakelandis
Copy link
Contributor Author

6.x backport: 679b0e1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants