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

Fix line length in org.elasticsearch.routing #37253

Merged
merged 2 commits into from
Jan 10, 2019

Conversation

pgomulka
Copy link
Contributor

@pgomulka pgomulka commented Jan 9, 2019

Remove the line length suppression for this package and fix offending
lines

relates: #34884

@pgomulka pgomulka self-assigned this Jan 9, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@pgomulka pgomulka force-pushed the fix/routing-line-length branch from 8dd770f to 260cead Compare January 9, 2019 09:22
Remove the line length suppression for this package and fix offending
lines

relates: elastic#34884
@pgomulka pgomulka force-pushed the fix/routing-line-length branch from 260cead to fafc1ba Compare January 9, 2019 21:26
@pgomulka
Copy link
Contributor Author

run default distro tests

.addAliasAction(AliasActions.add().index("test").alias("alias0").routing("0"))
.addAliasAction(AliasActions.add().index("test").alias("alias1").routing("1"))
.addAliasAction(AliasActions.add().index("test").alias("alias01").searchRouting("0,1")));
.addAliasAction(AliasActions.add().index("test").alias("alias"))
Copy link
Contributor

Choose a reason for hiding this comment

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

The original indentation seems what is used throughout the rest of code. Does it make sense to use the original style vs indenting under the period of the line above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer to change indentation for this file with fluent builder wrapping on dots.

Some of these chains are breaking the line length limit, I have reformatted them with intellij Allign when multiline. And then to keep this consistent across this file I formatted other fluent builders as well.

Copy link
Contributor

@ebadyano ebadyano left a comment

Choose a reason for hiding this comment

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

lgtm

@pgomulka pgomulka merged commit c812e6a into elastic:master Jan 10, 2019
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Jan 10, 2019
Remove the line length suppression for this package and fix offending
lines
backport elastic#37253

relates: elastic#34884
pgomulka added a commit that referenced this pull request Jan 11, 2019
Remove the line length suppression for this package and fix offending
lines
backport #37253

relates: #34884
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.

4 participants