too many lines too long
Ömer Sinan Ağacan
omeragacan at gmail.com
Mon Nov 9 22:17:22 UTC 2015
I also dislike the idea of automatically rejecting such code. I agree with
Austin's argument that the contribution barrier is already too high and
Richard's arguments, but in addition to those, I think it wouldn't be fair
because some patches of people with push access won't be subject to the
automatic lint checks. 100-col lines will make it to the code base even if only
by mistake. It'll be annoying to new contributors and won't solve the problem.
Personally I'm trying to be very careful, and in my patches I usually do a lint
pass at the end to fix all long lines. But I'm OK if some patches with 100-col
lines occasionally make it to the master.
2015-11-09 16:21 GMT-05:00 Richard Eisenberg <eir at cis.upenn.edu>:
> I agree that being forceful about the 80-col limit would solve my problem.
>
> But I really dislike the idea. There will always be long-running patches. Volunteers can't be relied on to have time available to continue their work right away. And so I think this decision would increase barriers to contributing and increase merge conflicts for a cause that, frankly, isn't terribly important. (To repeat: I *do* want 80-col lines. I just want an amazing compiler more.)
>
> Richard
>
> On Nov 9, 2015, at 4:15 PM, Austin Seipp <austin at well-typed.com> wrote:
>
>> Something like this might be possible. It'd just require implementing
>> a new arcanist linter, I think, and enabling it in .arclint
>>
>> In general I really sympathize with this. The problem 90% of people
>> hit is that they touch a line that was *already* over 80 columns, so
>> 'arc lint' warns them and gets annoyed, but they don't want to fix or
>> split up a bunch of stuff to avoid it. It's an issue of having to do
>> 'boring work' which nobody likes, and seems very tedious, regardless
>> of the mechanism of how they do the change.
>>
>> Really, I'm more inclined to begin a policy of rejecting reviews that
>> do not pass the linter. Exceptions can be made, but in general we need
>> to start *enforcing it* with the red button I think. And it would
>> require us to be more diligent about merging patches quickly to reduce
>> the scope of merge conflicts (because fixing an 80col violation
>> normally, almost always, adds more LOC).
>>
>> However, there are people who in general think the contribution
>> barrier is already too high, and I fear that enforcing this with a
>> hard rule may make people 'give up' because it seems like a pointless
>> thing to mandate to block their changes. I'm not sure how people feel
>> about that, but it is worth keeping in mind the developer economics.
>>
>> I hope suggesting the possibility of being more forceful against 80col
>> violations doesn't derail this too much. :)
>>
>> On Mon, Nov 9, 2015 at 3:02 PM, Richard Eisenberg <eir at cis.upenn.edu> wrote:
>>> Hi devs,
>>>
>>> We seem to be uncommitted to the ideal of 80-character lines. Almost every patch on Phab I look through has a bunch of "line too long" lint errors. No one seems to do much about these. And Phab's very very loud indication of a lint error makes reviewing the code harder.
>>>
>>> I like the ideal of 80-character lines. I aim for this ideal in my patches, falling short sometimes, of course. But I think the current setting of requiring everyone to "explain" away their overlong lines during `arc diff` and then trying hard to ignore the lint errors during code review is wrong. And it makes us all inured to more serious lint errors.
>>>
>>> How about this: 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?
>>>
>>> Would this be ignored more or followed more? Who knows. But it would sure be less annoying. :)
>>>
>>> What do others think?
>>>
>>> Thanks,
>>> Richard
>>> _______________________________________________
>>> ghc-devs mailing list
>>> ghc-devs at haskell.org
>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
>>>
>>
>>
>>
>> --
>> Regards,
>>
>> Austin Seipp, Haskell Consultant
>> Well-Typed LLP, http://www.well-typed.com/
>>
>
> _______________________________________________
> ghc-devs mailing list
> ghc-devs at haskell.org
> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
More information about the ghc-devs
mailing list