[GHC] #11082: Tweak workflow around overlong lines
#11082: Tweak workflow around overlong lines -------------------------------------+------------------------------------- Reporter: goldfire | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Build System | Version: 7.10.2 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- [https://mail.haskell.org/pipermail/ghc-devs/2015-November/010373.html 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. :) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11082 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11082: Tweak workflow around overlong lines -------------------------------------+------------------------------------- Reporter: goldfire | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Build System | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by rwbarton): I also like your original proposal. This technique is known as ratcheting (http://chrisstevenson.me/development/2008/03/11/fixing-broken-windows- with-ratcheting.html). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11082#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11082: Tweak workflow around overlong lines -------------------------------------+------------------------------------- Reporter: goldfire | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Build System | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): In the module I'm editing today `rnStmtsWithFreeVars` has 18 characters, or 25% of my total column budget, not counting the (essential) indentation. I hear what you say about laptop screens, and I'll accept the Will of the majority, but 80 still seems terribly short to me. In these days of wide- screen format for movies, can you really not get more than two columns of 80 chars? Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11082#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11082: Tweak workflow around overlong lines -------------------------------------+------------------------------------- Reporter: goldfire | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Build System | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by jstolarek): Copy-paste from ghc-devs (since Richard requested): Firstly, I hate explaining myself to Arcanist. When prompted to explain the reason for too long lines I typically enter "wontfix" without thinking too much. Secondly, I really don't like how warnings clutter code reviews. I have my Emacs highlight text beyond 80th column with a really ugly colour, so I strive real hard to maintain 80-column limit whenever possible. But sometimes fitting in that limit is nearly impossible: imagine being in a let nested in a do-notation nested in a guard nested in a where clause. Approx. 15-20 columns are lost for the indentation. Nevertheless I would support introducing a hard limit on having no more than 80 columns. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11082#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11082: Tweak workflow around overlong lines -------------------------------------+------------------------------------- Reporter: goldfire | Owner: Type: task | Status: new Priority: normal | Milestone: Component: Build System | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by diatchki): I'd also vote for formatting that fits in 80 characters. I typically work with two editors open next to each other: usually one for editing code, and one for browsing code, and it is really nice not to have to constantly scroll or switch between windows. I follow the 80 character limit on all projects I work with, and I find that it is not very hard to format code to fit in the window, even when there are long name. To do that I either use more but shorter lines, or I name large sub-expressions. I think both of those contribute to the readability of the code. Of course, that's quite subjective. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11082#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#11082: Tweak workflow around overlong lines -------------------------------------+------------------------------------- Reporter: goldfire | Owner: Type: task | Status: closed Priority: normal | Milestone: Component: Build System | Version: 7.10.2 Resolution: wontfix | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by thomie): * status: new => closed * resolution: => wontfix Comment: This is not a build system issue. Please take your quibbles to [https://phabricator.haskell.org/project/view/2/ Phabricator], where Austin can see them. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11082#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC