Re: checkstyle changes

2018-01-16 Thread Zoltan Haindrich
Well okay..but I still feel that checking for 100 kinda takes the value of these checks away; because people will just ignore the checks because they are red all the timeso I still think it should be raised to some higher value. Would 120 be ok? Choosing a higher linelength does not meab that

Re: checkstyle changes

2017-12-07 Thread Rui Li
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 wrote: > Problem with 140-wide code isn't just laptops - in many cases we need to do > sid

Re: checkstyle changes

2017-12-07 Thread Alexander Kolbasov
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 wrote: > I think the 140-character change will make the code hard to use on a > lap

Re: checkstyle changes

2017-12-07 Thread Sergey Shelukhin
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" wrote: >Disclaimer: I did not have time to test it out, but according to >http://checkstyle.sourceforge.net/config_misc.html#Indentation >

Re: checkstyle changes

2017-12-07 Thread Peter Vary
Disclaimer: I did not have time to test it out, but according to http://checkstyle.sourceforge.net/config_misc.html#Indentation Maybe the indentation could be solved by: lineWrappingIndentation=2 (default 4) forceStrictCondition=fal

Re: checkstyle changes

2017-12-07 Thread Zoltan Haindrich
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 m

Re: checkstyle changes

2017-12-06 Thread Eugene Koifman
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

Re: checkstyle changes

2017-12-05 Thread Peter Vary
+1 for the changes > On Dec 5, 2017, at 1:02 PM, Zoltan Haindrich 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

checkstyle changes

2017-12-05 Thread Zoltan Haindrich
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 / et