On Fri, Jun 23, 2017 at 6:03 PM, Oliver Heger <oliver.he...@oliver-heger.de> wrote:
> > > Am 23.06.2017 um 08:58 schrieb Allon Mureinik: > > The root cause, IMHO, is having failValidation=false configured in the > > pom.xml. This way, when you introduce a new problem your only option to > > notice it is if you visually scan mvn's output. As evident by the current > > state of the build, not everyone notices these. > > A more robust approach would be to set failValidation=true, and actively > > fail the build if checkstyle's rules are violated. > > > > I've submitted a PR to fix all the existing issues and enable this > > validation. Reviews are welcome: > > https://github.com/apache/commons-configuration/pull/5 > > > > Thanks for the PR, I will have a look. > > However, letting the build fail because of checkstyle error is too > restrictive IMHO. My approach is to work through the errors before > creating a new release. This has the disadvantage that errors might > accumulate; but from one release to the next one there is typically not > that much. > > Oliver > This is ultimately a matter of taste, but let me try to explain this point of view better. The baseline is that the project should pass checkstyle with no issues (the first three patches in this PR will get us there). >From that point on, the goal is not accept any patch that would break the styling. Think of it way - If you were reviewing a patch that didn't comply to the project's style, you'd comment about it and ask the author to fix it before merging. Having checkstyle do this *as part of the CI* has the same effect, but it takes some responsibility off the maintainers' shoulders. First, contributers can easily see if they need to improve their patch by running mvn install (arguably, they could do this even without enabling checkstyle validations, but it makes it much easier to notice). Second, and more importantly, it allows the CI to block such patches, so maintainers can decide whether to reject them (or even fix them themselves, if they are so inclined) so that checking checkstyle before the release becomes a non-issue - it just always passes. > > > > > On Thu, Jun 22, 2017 at 11:10 PM, Gary Gregory <garydgreg...@gmail.com> > > wrote: > > > >> FYI, to whom can take the time to fix this. > >> > >> When I run 'mvn clean install', I get: > >> > >> [INFO] --- maven-checkstyle-plugin:2.15:check (default) @ > >> commons-configuration2 --- > >> [INFO] There are 23 errors reported by Checkstyle 6.1.1 with > >> C:\vcs\svn\apache\commons\trunks-proper\configuration/conf/ > checkstyle.xml > >> ruleset. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> AbstractHierarchicalConfiguration.java[976] > >> (regexp) RegexpSingleline: Line has trailing spaces. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> AbstractHierarchicalConfiguration.java[978:30] > >> (blocks) LeftCurly: '{' should be on a new line. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> AbstractYAMLBasedConfiguration.java[0] > >> (misc) NewlineAtEndOfFile: File does not end with a newline. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\builder\fluent\ > >> INIBuilderParameters.java[0] > >> (misc) NewlineAtEndOfFile: File does not end with a newline. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\builder\ > >> INIBuilderParametersImpl.java[0] > >> (misc) NewlineAtEndOfFile: File does not end with a newline. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\builder\ > >> INIBuilderParametersImpl.java[42:5] > >> (whitespace) FileTabCharacter: File contains tab characters (this is the > >> first instance). > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\builder\ > >> INIBuilderParametersImpl.java[52:84] > >> (blocks) LeftCurly: '{' should be on a new line. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> builder\INIBuilderProperties.java[0] > >> (misc) NewlineAtEndOfFile: File does not end with a newline. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ex\ > >> ConfigurationRuntimeException.java[68] > >> (regexp) RegexpSingleline: Line has trailing spaces. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\JSONConfigur > ation.java[0] > >> (misc) NewlineAtEndOfFile: File does not end with a newline. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> JSONConfiguration.java[43:5] > >> (javadoc) JavadocVariable: Missing a Javadoc comment. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> JSONConfiguration.java[44:5] > >> (javadoc) JavadocVariable: Missing a Javadoc comment. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\tree\ > >> ImmutableNode.java[106] > >> (regexp) RegexpSingleline: Line has trailing spaces. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\tree\ > >> ImmutableNode.java[114:27] > >> (blocks) LeftCurly: '{' should be on a new line. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\tree\ > >> ImmutableNode.java[117] > >> (regexp) RegexpSingleline: Line has trailing spaces. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\tree\ > >> ImmutableNode.java[666] > >> (regexp) RegexpSingleline: Line has trailing spaces. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> XMLConfiguration.java[1169:15] > >> (whitespace) WhitespaceAround: 'if' is not followed by whitespace. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> XMLConfiguration.java[1210:15] > >> (whitespace) WhitespaceAround: 'if' is not followed by whitespace. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> XMLConfiguration.java[1212:19] > >> (whitespace) WhitespaceAround: 'if' is not followed by whitespace. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\ > >> XMLConfiguration.java[1311:20] > >> (whitespace) WhitespaceAround: 'if' is not followed by whitespace. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\XMLListRefer > ence.java[0] > >> (misc) NewlineAtEndOfFile: File does not end with a newline. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\XMLListRefer > ence.java[45] > >> (design) FinalClass: Class XMLListReference should be declared as final. > >> [ERROR] > >> src\main\java\org\apache\commons\configuration2\YAMLConfigur > ation.java[0] > >> (misc) NewlineAtEndOfFile: File does not end with a newline. > >> [WARNING] checkstyle:check violations detected but failOnViolation set > to > >> false > >> > >> Gary > >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
