[
https://issues.apache.org/jira/browse/GIRAPH-231?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13407661#comment-13407661
]
Eli Reisman commented on GIRAPH-231:
------------------------------------
Hoping to kickstart a conversation here as this is a topic that I'm sure
everyone has opinions on and some sort of adjustment to checkstyle would
probably be welcome. For the record, my naive opinion is this:
1. 80 chars per line and enforcing clear spacing between tokens is really great
to me
2. chasing after trailing whitespace and certain kinds of strict indentation
rules (4 or less spaces is specific enough) make life challenging when you
might use different IDE's at different times or on different machines that do
some default formatting when you open/save a file and then you have to chase
around on your screen editor fixing stuff that was not broken before you opened
the file.
In short, anything that encourages/enforces code clarity is great, anything
that causes more pain than it does correct clarity reduces the efficiency and
speed one can apply to a problem when they might have limited time to work on
it. Those moments feel like a trade-off that doesn't buy us anything.
Thats my two cents. I am not firmly anti-checkstyle by any means, but a
discussion about relaxing some of the guidelines is definately a conversation
worth having.
> 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
