Am 23.06.2017 um 17:24 schrieb Allon Mureinik:
> 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.
>
>
The fixes for checkstyle issues have been applied. Many thanks again.
Regarding the change of the checkstyle failure behavior, I prefer
waiting for a more global decision.
Oliver
>
>>
>>>
>>> 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
>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
