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

reevaluate tokens in google config for OperatorWrapCheck #3749

Closed
rnveach opened this Issue Jan 23, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@rnveach
Member

rnveach commented Jan 23, 2017

Taken from PR #3743,
When reviewing the tokens of OperatorWrapCheck, it is unclear if we should add the following tokens to google configuration.

"DIV_ASSIGN", "BOR_ASSIGN", "SL_ASSIGN", "ASSIGN", "BSR_ASSIGN", "BAND_ASSIGN", "PLUS_ASSIGN", "MINUS_ASSIGN", "SR_ASSIGN", "STAR_ASSIGN", "BXOR_ASSIGN", "MOD_ASSIGN"

We should review them and ask google for clarification.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 25, 2017

Member

All the tokens in question are assignment operators.

http://checkstyle.sourceforge.net/reports/google-java-style-20160712.html#s4.5.1-line-wrapping-where-to-break

When a line is broken at an assignment operator the break typically comes after the symbol, but either way is acceptable.

Before or after is acceptable, but they prefer after the symbol.
@romani How do we want to handle this? Just ignore the tokens since either method is acceptable? Or do we lead them to preferred method?

Member

rnveach commented Jan 25, 2017

All the tokens in question are assignment operators.

http://checkstyle.sourceforge.net/reports/google-java-style-20160712.html#s4.5.1-line-wrapping-where-to-break

When a line is broken at an assignment operator the break typically comes after the symbol, but either way is acceptable.

Before or after is acceptable, but they prefer after the symbol.
@romani How do we want to handle this? Just ignore the tokens since either method is acceptable? Or do we lead them to preferred method?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 26, 2017

Member

if acceptable it means allowed.
checkstyle can not satisfy "preferred" term at all. In checkstyle it is either allowed or forbidden.

Member

romani commented Jan 26, 2017

if acceptable it means allowed.
checkstyle can not satisfy "preferred" term at all. In checkstyle it is either allowed or forbidden.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 26, 2017

Member

@romani Then for this issue, we will not add the tokens or change the configuration and and just update the reason in the test, correct?

Member

rnveach commented Jan 26, 2017

@romani Then for this issue, we will not add the tokens or change the configuration and and just update the reason in the test, correct?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 26, 2017

Member

yes.
we might add this cases in ITs to enforce this a bit firmly.

Member

romani commented Jan 26, 2017

yes.
we might add this cases in ITs to enforce this a bit firmly.

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