I also believe 140 is a little too long. BTW do we use 2 or 4 chars for continuation indent? I personally prefer 4, but I do find both cases in out code.
On Fri, Dec 8, 2017 at 6:20 AM, Alexander Kolbasov <ak...@cloudera.com> wrote: > Problem with 140-wide code isn't just laptops - in many cases we need to do > side-by-side diffs (e.g. for code reviews) and this doubles the required > size. > > - Alex. > > On Thu, Dec 7, 2017 at 1:38 PM, Sergey Shelukhin <ser...@hortonworks.com> > wrote: > > > I think the 140-character change will make the code hard to use on a > > laptop without a monitor. > > > > > > On 17/12/7, 02:43, "Peter Vary" <pv...@cloudera.com> wrote: > > > > >Disclaimer: I did not have time to test it out, but according to > > >http://checkstyle.sourceforge.net/config_misc.html#Indentation > > ><http://checkstyle.sourceforge.net/config_misc.html#Indentation> > > >Maybe the indentation could be solved by: > > >lineWrappingIndentation=2 (default 4) > > >forceStrictCondition=false (default false) > > > > > >http://checkstyle.sourceforge.net/config_misc.html#TrailingComment > > ><http://checkstyle.sourceforge.net/config_misc.html#TrailingComment> > > >might help with the comments > > > > > >Sorry for not being more helpful. Maybe sometime later I will have time > > >to check these out. > > > > > >Thanks, > > >Peter > > > > > >> On Dec 7, 2017, at 10:26 AM, Zoltan Haindrich > > >><zhaindr...@hortonworks.com> wrote: > > >> > > >> Hello Eugene! > > >> > > >> I've looked into doing something with these; but I was not able to > > >>relieve the warnings you've mentioned: > > >> > > >> * the ;// is seems to be not configurable > > >> It seems like its handled by the whitespaceafter module; I'm not > sure > > >>how to allow / after ; > > >> * I think that indentation of 4 for many method arguments makes it > more > > >>readable; so I think it would be the best to just drop this check...but > > >>I've not seen any way to do this(w/o disabling the whole indentation > > >>module...) > > >> > > >> maybe someone else should take a look at it....I find it pretty hard > to > > >>get docs about specific chechkstyle configurations; since the search > > >>keywords mostly contain keywords like: semicolon, whitespace, > > >>comment...which tends to pull in all kind of garbage results :) > > >> > > >> cheers, > > >> Zoltan > > >> > > >> On 6 Dec 2017 8:53 p.m., Eugene Koifman <ekoif...@hortonworks.com> > > >>wrote: > > >> It currently complains about no space between ; and // as in > “…);//foo” > > >> > > >> And also about indentation when a single method call is split into > > >>multiple lines. > > >> It insists on 4 chars in this case, though we use 2 in (all?) other > > >>cases. > > >> > > >> Could this be dialed down as well? > > >> > > >> > > >> On 12/5/17, 7:26 AM, "Peter Vary" <pv...@cloudera.com> wrote: > > >> > > >> +1 for the changes > > >> > > >>> On Dec 5, 2017, at 1:02 PM, Zoltan Haindrich <k...@rxd.hu> wrote: > > >>> > > >>> Hello, > > >>> > > >>> I've filed a ticket to make the checkstyle warnings less noisy > > >>>(https://issues.apache.org/jira/browse/HIVE-18222) > > >>> > > >>> * set maxlinelength to 140 > > >>> I think everyone is working with big-enough displays to handle this > > >>>:) > > >>> There are many methods which have complicated names / arguments / > > >>>etc ; breaking the lines more frequently hurts readability... > > >>> * disabled some restrictions like: declaration&hiding via get/set > > >>>methods for protected/package fields are not mandatory > > >>> > > >>> If you don't feel comfortable with these changes, please share your > > >>>point of view. > > >>> > > >>> cheers, > > >>> Zoltan > > >>> > > >>> > > >> > > >> > > >> > > >> > > > > > > > > -- Best regards! Rui Li
