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

Enforce 140 char line lengths for packages action.bulk/delete/explain/get/index #34885

Merged
merged 6 commits into from
Oct 26, 2018

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.

@@ -46,7 +46,8 @@
* @see org.elasticsearch.client.Client#delete(DeleteRequest)
* @see org.elasticsearch.client.Requests#deleteRequest(String)
*/
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