hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Vary <pv...@cloudera.com>
Subject Re: checkstyle changes
Date Thu, 07 Dec 2017 10:43:41 GMT
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 <zhaindrich@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 <ekoifman@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" <pvary@cloudera.com> wrote:
> 
>    +1 for the changes
> 
>> On Dec 5, 2017, at 1:02 PM, Zoltan Haindrich <kirk@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
>> 
>> 
> 
> 
> 
> 


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