Skip to content

Fix line length in org.elasticsearch.routing#37253

Merged
pgomulka merged 2 commits intoelastic:masterfrom
pgomulka:fix/routing-line-length
Jan 10, 2019
Merged

Fix line length in org.elasticsearch.routing#37253
pgomulka merged 2 commits intoelastic:masterfrom
pgomulka:fix/routing-line-length

Conversation

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