Jakob Homan created GIRAPH-231:
----------------------------------
Summary: 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
