You are viewing a plain text version of this content. The canonical link for it is here.
Posted to yarn-dev@hadoop.apache.org by Shane Kumpf <sh...@gmail.com> on 2016/10/19 13:52:54 UTC
[DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
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.
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)?
Please share your thoughts.
Thank you,
Shane Kumpf
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Shane Kumpf <sh...@gmail.com>.
Thank you to everyone for the discussion.
To summarize, it appears there are no objections with moving forward
with HADOOP-13603,
which will remove the package line length checkstyle rule. Removing or
expanding the 80 character line length limit globally will not be changed
at this time.
Suppression of other checkstyle rules can be accomplished via annotations,
where appropriate, as of HADOOP-13411 (thank you for this work, it will be
quite useful!), so additional rule changes are not warranted at this time.
I will move forward on HADOOP-13603. Please continue to share feedback
and/or let me know if you disagree with the summary above.
Thank you!
-Shane Kumpf
On Fri, Oct 21, 2016 at 12:49 PM, Andrew Wang <an...@cloudera.com>
wrote:
> 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 <aa...@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 <jz...@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 <sh...@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
> >>>>
> >>>>
> >>>>
> >>>
> >>
> >
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Shane Kumpf <sh...@gmail.com>.
Thank you to everyone for the discussion.
To summarize, it appears there are no objections with moving forward
with HADOOP-13603,
which will remove the package line length checkstyle rule. Removing or
expanding the 80 character line length limit globally will not be changed
at this time.
Suppression of other checkstyle rules can be accomplished via annotations,
where appropriate, as of HADOOP-13411 (thank you for this work, it will be
quite useful!), so additional rule changes are not warranted at this time.
I will move forward on HADOOP-13603. Please continue to share feedback
and/or let me know if you disagree with the summary above.
Thank you!
-Shane Kumpf
On Fri, Oct 21, 2016 at 12:49 PM, Andrew Wang <an...@cloudera.com>
wrote:
> 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 <aa...@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 <jz...@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 <sh...@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
> >>>>
> >>>>
> >>>>
> >>>
> >>
> >
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Shane Kumpf <sh...@gmail.com>.
Thank you to everyone for the discussion.
To summarize, it appears there are no objections with moving forward
with HADOOP-13603,
which will remove the package line length checkstyle rule. Removing or
expanding the 80 character line length limit globally will not be changed
at this time.
Suppression of other checkstyle rules can be accomplished via annotations,
where appropriate, as of HADOOP-13411 (thank you for this work, it will be
quite useful!), so additional rule changes are not warranted at this time.
I will move forward on HADOOP-13603. Please continue to share feedback
and/or let me know if you disagree with the summary above.
Thank you!
-Shane Kumpf
On Fri, Oct 21, 2016 at 12:49 PM, Andrew Wang <an...@cloudera.com>
wrote:
> 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 <aa...@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 <jz...@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 <sh...@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
> >>>>
> >>>>
> >>>>
> >>>
> >>
> >
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Shane Kumpf <sh...@gmail.com>.
Thank you to everyone for the discussion.
To summarize, it appears there are no objections with moving forward
with HADOOP-13603,
which will remove the package line length checkstyle rule. Removing or
expanding the 80 character line length limit globally will not be changed
at this time.
Suppression of other checkstyle rules can be accomplished via annotations,
where appropriate, as of HADOOP-13411 (thank you for this work, it will be
quite useful!), so additional rule changes are not warranted at this time.
I will move forward on HADOOP-13603. Please continue to share feedback
and/or let me know if you disagree with the summary above.
Thank you!
-Shane Kumpf
On Fri, Oct 21, 2016 at 12:49 PM, Andrew Wang <an...@cloudera.com>
wrote:
> 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 <aa...@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 <jz...@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 <sh...@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
> >>>>
> >>>>
> >>>>
> >>>
> >>
> >
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Andrew Wang <an...@cloudera.com>.
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 <aa...@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 <jz...@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 <st...@hortonworks.com>
>>> wrote:
>>>
>>>
>>>> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>>>>
>>>>
>>>>
>>>
>>
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Andrew Wang <an...@cloudera.com>.
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 <aa...@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 <jz...@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 <st...@hortonworks.com>
>>> wrote:
>>>
>>>
>>>> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>>>>
>>>>
>>>>
>>>
>>
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Andrew Wang <an...@cloudera.com>.
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 <aa...@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 <jz...@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 <st...@hortonworks.com>
>>> wrote:
>>>
>>>
>>>> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>>>>
>>>>
>>>>
>>>
>>
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Andrew Wang <an...@cloudera.com>.
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 <aa...@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 <jz...@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 <st...@hortonworks.com>
>>> wrote:
>>>
>>>
>>>> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>>>>
>>>>
>>>>
>>>
>>
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle
rule
Posted by Akira Ajisaka <aa...@apache.org>.
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/3e1785cbbe14dcab9bb970fa0f534811cfe00795a8cd1100580f27dc@1430849118@%3Ccommon-dev.hadoop.apache.org%3E
>
> On Thu, Oct 20, 2016 at 7:46 AM, John Zhuge <jz...@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 <st...@hortonworks.com>
>> wrote:
>>
>>>
>>>> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>>>
>>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-dev-help@hadoop.apache.org
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle
rule
Posted by Akira Ajisaka <aa...@apache.org>.
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/3e1785cbbe14dcab9bb970fa0f534811cfe00795a8cd1100580f27dc@1430849118@%3Ccommon-dev.hadoop.apache.org%3E
>
> On Thu, Oct 20, 2016 at 7:46 AM, John Zhuge <jz...@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 <st...@hortonworks.com>
>> wrote:
>>
>>>
>>>> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>>>
>>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: common-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-dev-help@hadoop.apache.org
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle
rule
Posted by Akira Ajisaka <aa...@apache.org>.
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/3e1785cbbe14dcab9bb970fa0f534811cfe00795a8cd1100580f27dc@1430849118@%3Ccommon-dev.hadoop.apache.org%3E
>
> On Thu, Oct 20, 2016 at 7:46 AM, John Zhuge <jz...@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 <st...@hortonworks.com>
>> wrote:
>>
>>>
>>>> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>>>
>>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: mapreduce-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: mapreduce-dev-help@hadoop.apache.org
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle
rule
Posted by Akira Ajisaka <aa...@apache.org>.
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/3e1785cbbe14dcab9bb970fa0f534811cfe00795a8cd1100580f27dc@1430849118@%3Ccommon-dev.hadoop.apache.org%3E
>
> On Thu, Oct 20, 2016 at 7:46 AM, John Zhuge <jz...@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 <st...@hortonworks.com>
>> wrote:
>>
>>>
>>>> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>>>
>>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-dev-help@hadoop.apache.org
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Andrew Wang <an...@cloudera.com>.
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/3e1785cbbe14dcab9bb970fa0f534811cfe00795a8cd1100580f27dc@1430849118@%3Ccommon-dev.hadoop.apache.org%3E
On Thu, Oct 20, 2016 at 7:46 AM, John Zhuge <jz...@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 <st...@hortonworks.com>
> wrote:
>
> >
> > > On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
> >
> >
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Andrew Wang <an...@cloudera.com>.
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/3e1785cbbe14dcab9bb970fa0f534811cfe00795a8cd1100580f27dc@1430849118@%3Ccommon-dev.hadoop.apache.org%3E
On Thu, Oct 20, 2016 at 7:46 AM, John Zhuge <jz...@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 <st...@hortonworks.com>
> wrote:
>
> >
> > > On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
> >
> >
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Andrew Wang <an...@cloudera.com>.
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/3e1785cbbe14dcab9bb970fa0f534811cfe00795a8cd1100580f27dc@1430849118@%3Ccommon-dev.hadoop.apache.org%3E
On Thu, Oct 20, 2016 at 7:46 AM, John Zhuge <jz...@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 <st...@hortonworks.com>
> wrote:
>
> >
> > > On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
> >
> >
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by Andrew Wang <an...@cloudera.com>.
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/3e1785cbbe14dcab9bb970fa0f534811cfe00795a8cd1100580f27dc@1430849118@%3Ccommon-dev.hadoop.apache.org%3E
On Thu, Oct 20, 2016 at 7:46 AM, John Zhuge <jz...@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 <st...@hortonworks.com>
> wrote:
>
> >
> > > On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
> >
> >
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by John Zhuge <jz...@cloudera.com>.
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 <st...@hortonworks.com>
wrote:
>
> > On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle rule
Posted by John Zhuge <jz...@cloudera.com>.
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 <st...@hortonworks.com>
wrote:
>
> > On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
>
>
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle
rule
Posted by Steve Loughran <st...@hortonworks.com>.
> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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: yarn-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-dev-help@hadoop.apache.org
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle
rule
Posted by Steve Loughran <st...@hortonworks.com>.
> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle
rule
Posted by Steve Loughran <st...@hortonworks.com>.
> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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: common-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-dev-help@hadoop.apache.org
Re: [DISCUSS] HADOOP-13603 - Remove package line length checkstyle
rule
Posted by Steve Loughran <st...@hortonworks.com>.
> On 19 Oct 2016, at 14:52, Shane Kumpf <sh...@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: mapreduce-dev-unsubscribe@hadoop.apache.org
For additional commands, e-mail: mapreduce-dev-help@hadoop.apache.org