hadoop-hdfs-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Wang <andrew.w...@cloudera.com>
Subject Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Date Fri, 21 Oct 2016 18:49:06 GMT
Thanks for the clarification Akira, I'm fine with removing it for the
package line too (and imports if that's a problem), +1.

On Fri, Oct 21, 2016 at 2:02 AM, Akira Ajisaka <aajisaka@apache.org> wrote:

> This discussion was split into two separate topics.
>
> 1) Remove line length checkstyle rule for package line
> 2) Remove line length checkstyle rule for the entire source code
>
> 1) I'm +1 for removing the rule for package line. I can provide a trivial
> patch shortly in HADOOP-13603.
>
> 2) As Andrew said, the discussion was done in 2015. If we really want to
> change the rule, we need to discuss again.
>
> Regards,
> Akira
>
>
> On 10/21/16 07:12, Andrew Wang wrote:
>
>> I don't think anything has really changed since we had this discussion in
>> 2015 [1]. Github and gerrit and IDEs existed then too, and we decided to
>> leave it at 80 characters due to split screens and readability.
>>
>> I personally still like 80 chars for these same reasons.
>>
>> [1]
>> https://lists.apache.org/thread.html/3e1785cbbe14dcab9bb970f
>> a0f534811cfe00795a8cd1100580f27dc@1430849118@%3Ccommon-dev.h
>> adoop.apache.org%3E
>>
>> On Thu, Oct 20, 2016 at 7:46 AM, John Zhuge <jzhuge@cloudera.com> wrote:
>>
>> With HADOOP-13411, it is possible to suppress any checkstyle warning with
>>> an annotation.
>>>
>>> In this case, just add the following annotation before the class or
>>> method:
>>>
>>>     @SuppressWarnings("checkstyle:linelength")
>>>
>>> However this will not work if the warning is widespread in different
>>> classes or methods.
>>>
>>> Thanks,
>>> John Zhuge
>>>
>>> John Zhuge
>>> Software Engineer, Cloudera
>>>
>>> On Thu, Oct 20, 2016 at 3:22 AM, Steve Loughran <stevel@hortonworks.com>
>>> wrote:
>>>
>>>
>>>> On 19 Oct 2016, at 14:52, Shane Kumpf <shane.kumpf.apache@gmail.com>
>>>>>
>>>> wrote:
>>>>
>>>>>
>>>>> All,
>>>>>
>>>>> I would like to start a discussion on the possibility of removing the
>>>>> package line length checkstyle rule (HADOOP-13603
>>>>> <https://issues.apache.org/jira/browse/HADOOP-13603>).
>>>>>
>>>>> While working on various aspects of YARN container runtimes, all of my
>>>>> pre-commit jobs would fail as the package line length exceeded 80
>>>>> characters. While I'm all for automated checks, I feel checks need to
>>>>>
>>>> be
>>>
>>>> enforceable and provide value. Fixing the package line length error
>>>>>
>>>> does
>>>
>>>> not improve readability or maintainability of the code, and IMO should
>>>>>
>>>> be
>>>
>>>> removed.
>>>>>
>>>>>
>>>> I kind of agree here
>>>>
>>>> working on other projects with wider line lenghts (100, 120) means that
>>>> you find going back to 80 chars so restrictive; and as we adopt java 8
>>>>
>>> code
>>>
>>>> with closures, your nesting gets even more complex. Trying to fit things
>>>> into 80 char width often adds lots of line breaks which can make the
>>>> code
>>>> messier than if it need be.
>>>>
>>>> the argument against wider lines has historically been "helped
>>>> side-by-side" patch reviews. But we have so much patch review software
>>>> these days: github, gerrit, IDEs. i don't think we need to stay in
>>>> punched-card width code limits just because it worked with a review
>>>>
>>> process
>>>
>>>> of 6+ years ago
>>>>
>>>>
>>>> While on this topic, are there other automated checks that are
>>>>>
>>>> difficult
>>>
>>>> to
>>>>
>>>>> enforce or you feel are not providing value (perhaps the 150 line
>>>>>
>>>> method
>>>
>>>> length)?
>>>>>
>>>>>
>>>> I like that as a warning sign of complexity...it's not a hard veto after
>>>> all.
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
>>>> For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org
>>>>
>>>>
>>>>
>>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message