Re: [configuration] Checkstyle settings

[Oliver Heger]

Hi,

Am 10.08.2016 um 16:05 schrieb Raviteja Lokineni:
> Do you guys need any help in this? I can volunteer my time to help push
> this release, just let me know what needs to be done.

thank you for the offering.

I think, after the latest fix of Dennis, the codebase is now ready to
cut another RC. This can only be done by a committer. I was pretty busy
the last days, but the next RC is now on top of my todo list. So I hope
that I can call for another vote soon.

Oliver

> 
> On Sun, Aug 7, 2016 at 10:12 AM, Dennis Kieselhorst  wrote:
> 
>> Am 06.08.2016 um 17:51 schrieb Emmanuel Bourg:
>>> Le 2/08/2016 à 21:17, Oliver Heger a écrit :
>>>
 Well, for me style is not that important. (We cannot even agree on a
 common style for the Commons project.) Therefore, seeing the violations
 in the report is sufficient for me.
>>> +1, the build shouldn't fail due to style issues.
>>>
>> Ok, failOnViolation is now set to false.
>>
>> Regards
>> Dennis
>>
>> -
>> 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

Re: [configuration] Checkstyle settings

[Raviteja Lokineni]

Do you guys need any help in this? I can volunteer my time to help push
this release, just let me know what needs to be done.

On Sun, Aug 7, 2016 at 10:12 AM, Dennis Kieselhorst  wrote:

> Am 06.08.2016 um 17:51 schrieb Emmanuel Bourg:
> > Le 2/08/2016 à 21:17, Oliver Heger a écrit :
> >
> >> Well, for me style is not that important. (We cannot even agree on a
> >> common style for the Commons project.) Therefore, seeing the violations
> >> in the report is sufficient for me.
> > +1, the build shouldn't fail due to style issues.
> >
> Ok, failOnViolation is now set to false.
>
> Regards
> Dennis
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>


-- 
*Raviteja Lokineni* | Business Intelligence Developer
TD Ameritrade

E: raviteja.lokin...@gmail.com

[image: View Raviteja Lokineni's profile on LinkedIn]

Re: [configuration] Checkstyle settings

[Dennis Kieselhorst]

Am 06.08.2016 um 17:51 schrieb Emmanuel Bourg:
> Le 2/08/2016 à 21:17, Oliver Heger a écrit :
>
>> Well, for me style is not that important. (We cannot even agree on a
>> common style for the Commons project.) Therefore, seeing the violations
>> in the report is sufficient for me.
> +1, the build shouldn't fail due to style issues.
>
Ok, failOnViolation is now set to false.

Regards
Dennis

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [configuration] Checkstyle settings

[Emmanuel Bourg]

Le 2/08/2016 à 21:17, Oliver Heger a écrit :

> Well, for me style is not that important. (We cannot even agree on a
> common style for the Commons project.) Therefore, seeing the violations
> in the report is sufficient for me.

+1, the build shouldn't fail due to style issues.

Emmanuel Bourg


-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [configuration] Checkstyle settings

[Oliver Heger]

Am 02.08.2016 um 08:54 schrieb Dennis Kieselhorst:
> Am 01.08.2016 um 21:31 schrieb Oliver Heger:
>> Am 31.07.2016 um 22:24 schrieb Matt Sicker:
>>> Fixing all the checkstyle errors first is kind of a prerequisite to
>>> enabling it by default.
>>>
>>> On 31 July 2016 at 15:10, Charles Honton  wrote:
>>>
 Why wouldn’t we want build to fail early if incorrect style is used?
>> In this special case, the build was broken on JDK 1.6 which is blocking
>> the configuration 2.1 release.
>>
>> In general, IMHO it is too strict to let the build fail because a
>> bracket is set incorrectly or something like this. It is fine if this
>> generates a warning, and these warnings can be fixed before a release.
>> But I do not want to be forced to fix all style violations at any time.
>> Especially, as such violations will creep in nevertheless (somebody does
>> a quick commit without running the full build) and then cause problems
>> in the future.
>>
> I've changed it after committing with wrong codestyle. Before the
> checkstyle config was just in the reporting section, so only just for
> site generation. When I was running mvn checkstyle:check before my
> commit, I got thousands of violations as a default ruleset was applied.
> 
> Now you immediately see during normal build that something is wrong. Of
> course the build was passing after the change. During the release build
> we just discovered that generated sources were checked as well when
> using Maven 3.2.5 and JDK 1.6, but not checked when using Maven 3.3.9
> and JDK 1.7/ 1.8. This is now fixed by an exclude for the generated sources.
> 
> I prefer that the build fails as warnings are often ignored. If we
> change it, we should at least fail on Jenkins (using a ci profile).

Well, for me style is not that important. (We cannot even agree on a
common style for the Commons project.) Therefore, seeing the violations
in the report is sufficient for me.

Other reports are IMHO more important, e.g. findbugs or RAT (for legal
reasons). Would we also let the build fail when here violations are found?

I fear that such strict rules will also be an obstacle for contributors:
Somebody wants to play with the code, tries something out, and the build
fails because of unrelated stuff.

Oliver

> 
> Regards
> Dennis
> 
> -
> 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

Re: [configuration] Checkstyle settings

[Dennis Kieselhorst]

Am 01.08.2016 um 21:31 schrieb Oliver Heger:
> Am 31.07.2016 um 22:24 schrieb Matt Sicker:
>> Fixing all the checkstyle errors first is kind of a prerequisite to
>> enabling it by default.
>>
>> On 31 July 2016 at 15:10, Charles Honton  wrote:
>>
>>> Why wouldn’t we want build to fail early if incorrect style is used?
> In this special case, the build was broken on JDK 1.6 which is blocking
> the configuration 2.1 release.
>
> In general, IMHO it is too strict to let the build fail because a
> bracket is set incorrectly or something like this. It is fine if this
> generates a warning, and these warnings can be fixed before a release.
> But I do not want to be forced to fix all style violations at any time.
> Especially, as such violations will creep in nevertheless (somebody does
> a quick commit without running the full build) and then cause problems
> in the future.
>
I've changed it after committing with wrong codestyle. Before the
checkstyle config was just in the reporting section, so only just for
site generation. When I was running mvn checkstyle:check before my
commit, I got thousands of violations as a default ruleset was applied.

Now you immediately see during normal build that something is wrong. Of
course the build was passing after the change. During the release build
we just discovered that generated sources were checked as well when
using Maven 3.2.5 and JDK 1.6, but not checked when using Maven 3.3.9
and JDK 1.7/ 1.8. This is now fixed by an exclude for the generated sources.

I prefer that the build fails as warnings are often ignored. If we
change it, we should at least fail on Jenkins (using a ci profile).

Regards
Dennis

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [configuration] Checkstyle settings

[Oliver Heger]

Am 31.07.2016 um 22:24 schrieb Matt Sicker:
> Fixing all the checkstyle errors first is kind of a prerequisite to
> enabling it by default.
> 
> On 31 July 2016 at 15:10, Charles Honton  wrote:
> 
>> Why wouldn’t we want build to fail early if incorrect style is used?

In this special case, the build was broken on JDK 1.6 which is blocking
the configuration 2.1 release.

In general, IMHO it is too strict to let the build fail because a
bracket is set incorrectly or something like this. It is fine if this
generates a warning, and these warnings can be fixed before a release.
But I do not want to be forced to fix all style violations at any time.
Especially, as such violations will creep in nevertheless (somebody does
a quick commit without running the full build) and then cause problems
in the future.

Oliver

>>
>> chas
>>
>>> On Jul 31, 2016, at 11:09 AM, Oliver Heger 
>> wrote:
>>>
>>> Hi,
>>>
>>> in revision 1742698 the checkstyle configuration has been changed. The
>>> log says "fixed checkstyle violations, updated to latest version of
>>> checkstyle-maven-plugin, ensure correct checkstyle configuration is
>>> applied in all cases".
>>>
>>> I think with the new configuration checkstyle is now run on every mvn
>>> install and causes the build to fail if there are checkstyle errors.
>>> This is probably not what we want. What was the reason for this update,
>>> i.e. under which circumstances was an incorrect checkstyle configuration
>>> used?
>>>
>>> Thanks
>>> Oliver
>>>
>>> -
>>> 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
>>
>>
> 
> 

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [configuration] Checkstyle settings

[Matt Sicker]

Fixing all the checkstyle errors first is kind of a prerequisite to
enabling it by default.

On 31 July 2016 at 15:10, Charles Honton  wrote:

> Why wouldn’t we want build to fail early if incorrect style is used?
>
> chas
>
> > On Jul 31, 2016, at 11:09 AM, Oliver Heger 
> wrote:
> >
> > Hi,
> >
> > in revision 1742698 the checkstyle configuration has been changed. The
> > log says "fixed checkstyle violations, updated to latest version of
> > checkstyle-maven-plugin, ensure correct checkstyle configuration is
> > applied in all cases".
> >
> > I think with the new configuration checkstyle is now run on every mvn
> > install and causes the build to fail if there are checkstyle errors.
> > This is probably not what we want. What was the reason for this update,
> > i.e. under which circumstances was an incorrect checkstyle configuration
> > used?
> >
> > Thanks
> > Oliver
> >
> > -
> > 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
>
>


-- 
Matt Sicker 

Re: [configuration] Checkstyle settings

[Charles Honton]

Why wouldn’t we want build to fail early if incorrect style is used?

chas

> On Jul 31, 2016, at 11:09 AM, Oliver Heger  
> wrote:
> 
> Hi,
> 
> in revision 1742698 the checkstyle configuration has been changed. The
> log says "fixed checkstyle violations, updated to latest version of
> checkstyle-maven-plugin, ensure correct checkstyle configuration is
> applied in all cases".
> 
> I think with the new configuration checkstyle is now run on every mvn
> install and causes the build to fail if there are checkstyle errors.
> This is probably not what we want. What was the reason for this update,
> i.e. under which circumstances was an incorrect checkstyle configuration
> used?
> 
> Thanks
> Oliver
> 
> -
> 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

[configuration] Checkstyle settings

[Oliver Heger]

Hi,

in revision 1742698 the checkstyle configuration has been changed. The
log says "fixed checkstyle violations, updated to latest version of
checkstyle-maven-plugin, ensure correct checkstyle configuration is
applied in all cases".

I think with the new configuration checkstyle is now run on every mvn
install and causes the build to fail if there are checkstyle errors.
This is probably not what we want. What was the reason for this update,
i.e. under which circumstances was an incorrect checkstyle configuration
used?

Thanks
Oliver

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org