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 <andrew.w...@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 <aajis...@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 <jzh...@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 < > ste...@hortonworks.com> > >>> wrote: > >>> > >>> > >>>> On 19 Oct 2016, at 14:52, Shane Kumpf <shane.kumpf.apa...@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-unsubscr...@hadoop.apache.org > >>>> For additional commands, e-mail: hdfs-dev-h...@hadoop.apache.org > >>>> > >>>> > >>>> > >>> > >> > > >
