On Fri, Jun 23, 2017 at 8:24 AM, Allon Mureinik <murei...@gmail.com> wrote:
> 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. > Over at HttpComponents, checkstyle can break the build and that helps keeps the code tidy. Gary > 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 > > > > >
