Tweak workflow around overlong lines
This thread discussed potential changes to the way the GHC dev workflow deals with overlong lines. This ticket summarizes the discussion.
Initial complaint: arc/Phab is too heavy-handed about overlong lines. Almost all patchers have to write a vacuous "explanation" for them, and the overlong line warnings clutter code reviews.
Initial suggestion: After arc diff
is run, it will count the number of overlong lines before and after the patch. If there are more after, have the last thing arc diff
outputs be a stern telling-off of the dev, along the lines of
Before your patch, 15 of the edited lines were over 80 characters.
Now, a whopping 28 of them are. Can't you do better? Please?
Reactions:
-
+1
-
Even the delta in overlong lines is bad, because perhaps a small refactor added to the length of an identifier.
-
We might just want to reject code with overlong lines. Then the problem would really go away. It favors one-time work over many-time maintenance burden. (Austin's point, +1 by Alexander Berntsen)
* This would cause more work and would increase the barrier to contribution. And sometimes it's appropriate/more readable to have overlong lines.
-
80 is too small. 120 (SPJ)? 132 (Ed Kmett)?
-
And a small limit discourages informative identifiers.
-
Just drop the column limit entirely.
-
Why 80? Because two 80-column buffers fit nicely side-by-side on small laptop screens. Three such columns fit nicely on monitors. Several (4 emails) echoed this point.
Bottom line: no one expressed liking the status quo (though Simon M called it "not terrible"). If we're to choose a limit, a choice of 80 seemed to appease the plurality of respondents. Two respondents advocate enforcing the limit. I count four respondents rejecting hard enforcement.
In my (surely biased) view, this means that we should implement my original proposal. :)
Trac metadata
Trac field | Value |
---|---|
Version | 7.10.2 |
Type | Task |
TypeOfFailure | OtherFailure |
Priority | normal |
Resolution | Unresolved |
Component | Build System |
Test case | |
Differential revisions | |
BlockedBy | |
Related | |
Blocking | |
CC | |
Operating system | |
Architecture |