[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[ https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16349985#comment-16349985 ] Eric Barnhill commented on NUMBERS-50: -- I think this was the same issue that was causing Jenkins to throw errors, and it is resolved now. > Clean checkstyle for Complex > > > Key: NUMBERS-50 > URL: https://issues.apache.org/jira/browse/NUMBERS-50 > Project: Commons Numbers > Issue Type: Bug >Reporter: Eric Barnhill >Priority: Trivial > > Clean up trailing whitespaces and other checkstyle violations in > commons-numbers-complex -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[ https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16302635#comment-16302635 ] Gilles commented on NUMBERS-50: --- bq. option is to change the subject and extend the scope to solve all the checkstyle errors. Sure, but it could be confusing to change the purpose meant by the original reporter. I think that it would be nicer to open a new one similar to [that page (for RNG)|https://issues.apache.org/jira/browse/RNG-23]. > Clean checkstyle for Complex > > > Key: NUMBERS-50 > URL: https://issues.apache.org/jira/browse/NUMBERS-50 > Project: Commons Numbers > Issue Type: Bug >Reporter: Eric Barnhill >Priority: Trivial > > Clean up trailing whitespaces and other checkstyle violations in > commons-numbers-complex -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[
https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16302320#comment-16302320
]
Amey Jadiye commented on NUMBERS-50:
bq. No, but it is not necessary to explicitly "suppress" it since running mvn
clean site does not report any error.
This looks very strange to me, curious how {{mvn clean verify}} is failing but
now {{mvn clean site}}
bq. Anyways, it looks like this report can be resolved since as of today there
is no checkstyle violations in the module commons-numbers-complex
hmm, I also feel to close this. one more option is to change the subject and
extend the scope to solve all the checkstyle errors.
> Clean checkstyle for Complex
>
>
> Key: NUMBERS-50
> URL: https://issues.apache.org/jira/browse/NUMBERS-50
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Eric Barnhill
>Priority: Trivial
>
> Clean up trailing whitespaces and other checkstyle violations in
> commons-numbers-complex
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[
https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16276173#comment-16276173
]
Gilles commented on NUMBERS-50:
---
bq. I have changed two lines in pom.xml
{{true}}{{true}}
This should not be done as part of this issue/PR.
bq. we dont really target included in checkstyle checks
No, but it is not necessary to explicitly "suppress" it since running {{mvn
clean site}} does not report any error.
Please _disable_ {{}} and remove the
"checkstyle-suppressions.xml" file; then run the above command to confirm that
we obtain the same behaviour.
If another maven command behaves differently, I'd think that odd; but I'd not
deem it a sufficient reason to add an otherwise non-necessary file.
Anyways, it looks like this report can be resolved since as of today there is
no checkstyle violations in the module {{commons-numbers-complex}}.
> Clean checkstyle for Complex
>
>
> Key: NUMBERS-50
> URL: https://issues.apache.org/jira/browse/NUMBERS-50
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Eric Barnhill
>Priority: Trivial
>
> Clean up trailing whitespaces and other checkstyle violations in
> commons-numbers-complex
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[
https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16276012#comment-16276012
]
Amey Jadiye commented on NUMBERS-50:
bq. Do you mean in directory commons-numbers-complex/target?
Yes, I was checking checkstyle error for only complex for now, for limitting
scope of this jira.
bq. When I call mvn site, CheckStyle does not report anything there! How do you
call the report generation in order to get a different result?
I have changed two lines in pom.xml
{{truetrue}}
not sure if you also did this on your local machine? I'm simply executing
{{mvn clean verify checkstyle:check}} here. afer this change I can't call {{mvn
site}} thats also failing. below is the short snip with reason. after adding
target to suppression I'm able to build complex clean.
{code}
[INFO] --- maven-checkstyle-plugin:2.17:check (default-cli) @
commons-numbers-complex ---
[INFO] There is 1 error reported by Checkstyle 6.11.2 with
/home/amey/work/opensource/apache/commons/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
ruleset.
[ERROR] target/maven-archiver/pom.properties:[1] (header) Header: Missing a
header - not enough lines in file.
[INFO]
[INFO] BUILD FAILURE
[INFO]
[INFO] Total time: 34.253 s
[INFO] Finished at: 2017-12-03T22:28:15+05:30
[INFO] Final Memory: 37M/453M
[INFO]
{code}
bq. I rather think that we should heed the suggestions from CheckStyle unless
there is a good reason.
Agreed, removed all except target and generated source which is important here,
we dont really target included in checkstyle checks ,I think.
bq. also tend to think that failing the build because of picky rules (like
trailing whitespace) may not be pleasant when developing.
I think enfourcing those rules are good, that cleans the code while developing
it self , atleast I'm in favour of it.
bq. here was a discussion about mandating that contributors should run a
specific command (with options that would then fail the build, namely if
CheckStyle is not happy) before submitting a pull request. But I don't recall
that it has been done.
Yes, I did that in commons-text and its working like charm, enforceing that
makes code clean before merge to trunk. will do that for commons-numbers after
we finish with all the cleanup activities which includes pmd, checkstyle,
findbug, rat etc ...
> Clean checkstyle for Complex
>
>
> Key: NUMBERS-50
> URL: https://issues.apache.org/jira/browse/NUMBERS-50
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Eric Barnhill
>Priority: Trivial
>
> Clean up trailing whitespaces and other checkstyle violations in
> commons-numbers-complex
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[
https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16275516#comment-16275516
]
Gilles commented on NUMBERS-50:
---
bq. the only issues I found was in target of the complex
Do you mean in directory {{commons-numbers-complex/target}}?
When I call {{mvn site}}, CheckStyle does not report anything there! How do
you call the report generation in order to get a different result?
In other modules, CheckStyle does report a few errors (e.g.
{{HideUtilityClassConstructor}}) that would become hidden with the new
suppressions. I rather think that we should heed the suggestions from
CheckStyle unless there is a good reason.
bq. other suppressions are valid
At first sight I wouldn't agree. Was there a ML discussion about those
suppressions?
bq. copied from commons text
Indeed, there are references to classes that don't exist in "Numbers"...
I also tend to think that failing the build because of picky rules (like
trailing whitespace) may not be pleasant when developing.
IIRC, there was a discussion about mandating that contributors should run a
specific command (with options that would then fail the build, namely if
CheckStyle is not happy) before submitting a pull request. But I don't recall
that it has been done.
> Clean checkstyle for Complex
>
>
> Key: NUMBERS-50
> URL: https://issues.apache.org/jira/browse/NUMBERS-50
> Project: Commons Numbers
> Issue Type: Bug
>Reporter: Eric Barnhill
>Priority: Trivial
>
> Clean up trailing whitespaces and other checkstyle violations in
> commons-numbers-complex
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[ https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16275413#comment-16275413 ] Amey Jadiye commented on NUMBERS-50: I'm sorry for being not descriptive, the only issues I found was in *target* of the complex other suppressions are valid and copied from commons text. so I would say there is no valid issues which are suppressed. > Clean checkstyle for Complex > > > Key: NUMBERS-50 > URL: https://issues.apache.org/jira/browse/NUMBERS-50 > Project: Commons Numbers > Issue Type: Bug >Reporter: Eric Barnhill >Priority: Trivial > > Clean up trailing whitespaces and other checkstyle violations in > commons-numbers-complex -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[ https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16275202#comment-16275202 ] Gilles commented on NUMBERS-50: --- I don't understand the purpose of the patch (it seems that errors should be fixed rather than suppressed). > Clean checkstyle for Complex > > > Key: NUMBERS-50 > URL: https://issues.apache.org/jira/browse/NUMBERS-50 > Project: Commons Numbers > Issue Type: Bug >Reporter: Eric Barnhill >Priority: Trivial > > Clean up trailing whitespaces and other checkstyle violations in > commons-numbers-complex -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[ https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16274785#comment-16274785 ] ASF GitHub Bot commented on NUMBERS-50: --- GitHub user ameyjadiye opened a pull request: https://github.com/apache/commons-numbers/pull/11 NUMBERS-50 : cleaning complex - start failing on voilation and added check-style suppression Complex is clean now, however I see there are some checkstyle bugs in other modules as well, those will be addressed in other PR looking at scope of _NUMBERS-50_ You can merge this pull request into a Git repository by running: $ git pull https://github.com/ameyjadiye/commons-numbers master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-numbers/pull/11.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #11 commit 815dd23e421f10562b2e4627d2774df76f1426de Author: Amey Jadiye Date: 2017-12-01T19:01:55Z NUMBERS-50 : start failing on voilation and added check-style suppression, complex is clean > Clean checkstyle for Complex > > > Key: NUMBERS-50 > URL: https://issues.apache.org/jira/browse/NUMBERS-50 > Project: Commons Numbers > Issue Type: Bug >Reporter: Eric Barnhill >Priority: Trivial > > Clean up trailing whitespaces and other checkstyle violations in > commons-numbers-complex -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (NUMBERS-50) Clean checkstyle for Complex
[ https://issues.apache.org/jira/browse/NUMBERS-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16164636#comment-16164636 ] Eric Barnhill commented on NUMBERS-50: -- For some reason I am not getting the html file I used to, only a bare XML. Is there something I should change? > Clean checkstyle for Complex > > > Key: NUMBERS-50 > URL: https://issues.apache.org/jira/browse/NUMBERS-50 > Project: Commons Numbers > Issue Type: Bug >Reporter: Eric Barnhill >Priority: Trivial > > Clean up trailing whitespaces and other checkstyle violations in > commons-numbers-complex -- This message was sent by Atlassian JIRA (v6.4.14#64029)
