[RNG] Checkstyle

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[RNG] Checkstyle

Artem Barger
Hi,

I've enabled checkstyle Jenkins plugin and executed checkstyle plugin on
the current code base and it resulted in a lot of high priority warning see
here:
https://builds.apache.org/view/Apache%20Commons/job/Commons_Rng/14/checkstyleResult/HIGH/

Now, I'm wondering whenever it gets a wrong checkstyle.xml file or there
are indeed that many warnings or checkstyle.xml has to be adjusted?

Best regards,
                      Artem Barger.
Reply | Threaded
Open this post in threaded view
|

Re: [RNG] Checkstyle

Gilles Sadowski
On Wed, 17 Aug 2016 09:43:41 +0300, Artem Barger wrote:
> Hi,
>
> I've enabled checkstyle Jenkins plugin and executed checkstyle plugin
> on
> the current code base and it resulted in a lot of high priority
> warning see
> here:
>
> https://builds.apache.org/view/Apache%20Commons/job/Commons_Rng/14/checkstyleResult/HIGH/

I had noticed that.
I don't know whether Jenkins applying its own checkstyle configuration
is a bug or feature.

> Now, I'm wondering whenever it gets a wrong checkstyle.xml file or
> there
> are indeed that many warnings or checkstyle.xml has to be adjusted?

Most of the warnings are either false positives (e.g. "not designed
for inheritance") or spurious (e.g. line length).
Also, insisting on "final" for method arguments is (IMHO) borderline
ridiculous for Java code since it is impossible to ensure
"constantness"
of the instance.
There several instances of "argument hides a field" but this is common
style in constructors.

Some are quite real: there are indeed too many "magic numbers" but this
is bound to be the case in a code that does a lot of bits manipulation.
I already defined many of them as class fields, but if there is only
one such use in a class, it sometimes becomes less clear than keeping
the magic number where it belongs.

I corrected some of the Javadoc ones concerning comments not ending
with
a punctuation. [I find that rule interesting but never could convince
other developers that comments should follow correct English syntax.]

I think that, for readability, space around operators should be indeed
be mandatory. [I've just fixed a few of those.]

Some rules applied in Jenkins contradict rules enforced locally (copied
from Commons Math), e.g. "OperatorWrapCheck".

Regards,
Gilles

>
> Best regards,
>                       Artem Barger.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [RNG] Checkstyle

Brent Worden-2
It appears is it not using the correct checkstyle ruleset.  From the build
output:

[INFO] *--- maven-checkstyle-plugin:2.17:checkstyle (default-cli) @
commons-rng ---
*[INFO] There are 702 errors reported by Checkstyle 6.11.2 with
sun_checks.xml ruleset.


Brent


On Wed, Aug 17, 2016 at 5:40 AM, Gilles <[hidden email]>
wrote:

> On Wed, 17 Aug 2016 09:43:41 +0300, Artem Barger wrote:
>
>> Hi,
>>
>> I've enabled checkstyle Jenkins plugin and executed checkstyle plugin on
>> the current code base and it resulted in a lot of high priority warning
>> see
>> here:
>>
>> https://builds.apache.org/view/Apache%20Commons/job/Commons_
>> Rng/14/checkstyleResult/HIGH/
>>
>
> I had noticed that.
> I don't know whether Jenkins applying its own checkstyle configuration
> is a bug or feature.
>
> Now, I'm wondering whenever it gets a wrong checkstyle.xml file or there
>> are indeed that many warnings or checkstyle.xml has to be adjusted?
>>
>
> Most of the warnings are either false positives (e.g. "not designed
> for inheritance") or spurious (e.g. line length).
> Also, insisting on "final" for method arguments is (IMHO) borderline
> ridiculous for Java code since it is impossible to ensure "constantness"
> of the instance.
> There several instances of "argument hides a field" but this is common
> style in constructors.
>
> Some are quite real: there are indeed too many "magic numbers" but this
> is bound to be the case in a code that does a lot of bits manipulation.
> I already defined many of them as class fields, but if there is only
> one such use in a class, it sometimes becomes less clear than keeping
> the magic number where it belongs.
>
> I corrected some of the Javadoc ones concerning comments not ending with
> a punctuation. [I find that rule interesting but never could convince
> other developers that comments should follow correct English syntax.]
>
> I think that, for readability, space around operators should be indeed
> be mandatory. [I've just fixed a few of those.]
>
> Some rules applied in Jenkins contradict rules enforced locally (copied
> from Commons Math), e.g. "OperatorWrapCheck".
>
> Regards,
> Gilles
>
>
>> Best regards,
>>                       Artem Barger.
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [RNG] Checkstyle

Artem Barger
On Wed, Aug 17, 2016 at 3:55 PM, Brent Worden <[hidden email]>
wrote:

> It appears is it not using the correct checkstyle ruleset.  From the build
> output:
>
> [INFO] *--- maven-checkstyle-plugin:2.17:checkstyle (default-cli) @
> commons-rng ---
> *[INFO] There are 702 errors reported by Checkstyle 6.11.2 with
> sun_checks.xml ruleset.
>

​Thanks, will take a look to fix it.​


Best regards,
                      Artem Barger.