Skip to content

[Style] Remove line length violations from ingest actions#34886

Merged
jdconrad merged 2 commits intoelastic:masterfrom
jdconrad:length
Oct 26, 2018
Merged

[Style] Remove line length violations from ingest actions#34886
jdconrad merged 2 commits intoelastic:masterfrom
jdconrad:length

Conversation

@jdconrad
Copy link
Copy Markdown
Contributor

@jdconrad jdconrad commented Oct 25, 2018

Removes all line length violations from ingest actions to get rid the need for checkstyle suppression in this package.

Parent issue: #34884

Copy link
Copy Markdown
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

@jdconrad
Copy link
Copy Markdown
Contributor Author

@hub-cap @dakrone Thanks for the reviews! Will commit as soon as CI passes.

@jdconrad jdconrad added the :Distributed/Ingest Node Execution or management of Ingest Pipelines label Oct 25, 2018
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra

Copy link
Copy Markdown
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.

LGTM. I left one thing I'd love for you to change but thanks for cleaning these up!


public class GetPipelineRequestBuilder extends MasterNodeReadOperationRequestBuilder<GetPipelineRequest, GetPipelineResponse, GetPipelineRequestBuilder> {
public class GetPipelineRequestBuilder
extends MasterNodeReadOperationRequestBuilder<GetPipelineRequest, GetPipelineResponse, GetPipelineRequestBuilder> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you indent this one more time so the class declaration doesn't line up with the class body?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is my pet complaint to IntelliJ users 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nik9000 I hear you! I just redid my system and forgot to disable Intellij loading the editorconfig file that is checked in. I wonder if other Intellij users aren't doing the same.

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.

6 participants