[
https://issues.apache.org/jira/browse/GIRAPH-231?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13405410#comment-13405410
]
Avery Ching commented on GIRAPH-231:
------------------------------------
I think it's reasonable to discuss this and I don't have a strong feeling
regarding the exact choices (well, okay, the 80 character lines is nice to
have). The main thing I care about is that its consistent and most of the
contributors are happy with it. Code uniformity is important to me as it is
confusing to have many different styles. I don't necessarily think that the
checkstyle choices we have made are the "best" ones, but overall they are
helping to keep the code uniform, which I appreciate.
I just don't want to see
{noformat}
for(int i=0;i<0;++i)
{noformat}
in one block and then
{noformat}
for (int i = 0; i < 0; ++i)
{noformat}
in another. This makes the code look messy and confuses contributors (well, at
least me).
> Overly prescriptive check-style requirements considered harmful
> ---------------------------------------------------------------
>
> Key: GIRAPH-231
> URL: https://issues.apache.org/jira/browse/GIRAPH-231
> Project: Giraph
> Issue Type: Bug
> Reporter: Jakob Homan
>
> The current checkstyle settings are extremely precise and an excellent
> codification of particular coding style preference. However, like in
> religion and politics, reasonable and thoughtful people can have
> disagreements and should be accommodating to each other. The current
> checkstyle requirements venture into a lot of territory where disagreements
> and common and often conflict with other styles that are perfectly
> reasonable. Right now one can generate a perfectly reasonable looking patch
> that then takes longer to make checkstyle than it did to create it.
> A few examples:
> * Whether or not a for or if statement has a space before its opening paren
> does not in any way make the code less or more readable or bug free. Either
> preference is valid.
> * Not every method or field requires javadoc. I trust every contributor (or,
> barring that, reviewer) to use their experience and judgment to determine if
> one is needed
> * 80 characters per line is a reasonable arbitrary limit. But so is 85 or
> 90. And 80 seems to cause a lot of lines to be cut off at very odd places
> from a readability standpoint. I trust every contributor (or, barring that,
> reviewer) to determine if the line will cause some huge system failure by
> going to 83 characters. For instance which is worse:
> {noformat}
> String timeUnitString = conf.get(GIRAPH_METRICS_DEFAULT_TIME_PERIOD,
> "SECONDS");
> {noformat}
> or
> {noformat}
> String timeUnitString = conf.get(GIRAPH_METRICS_DEFAULT_TIME_PERIOD,
> "SECONDS");
> {noformat}
> Everybody has a preference on each of the items above, but I don't think
> anyone can reasonably make an argument that another's preference leads to
> more bugs or is objectively bad.
> Overly strict checkstyle settings, which is what I think we've ended up with,
> don't actually end up improving the readability of the code. Instead, they
> add a large amount of friction between contributors. If I spend most of my
> time in using another style that doesn't allow spaces between if and (, I
> find it painful and frustrating to try to contribute to this project.
> Readability is not something that can be guaranteed by any checkstyle
> configuration and instead, we should loosen the requirements and trust our
> contributors and reviewers to keep a good eye out for subtle errors and leave
> checkstyle to police the egregious ones.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
