commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject [ALL] Automated requirements (e.g. CheckStyle)? (Was: [MATH] Enforce run [...])
Date Mon, 07 Aug 2017 09:16:13 GMT
Hello.

On Fri, 4 Aug 2017 21:17:43 +0300, Allon Mureinik wrote:
> We had a similar discussion about Configuration.
>
> Personally, I'm all for enforcing checkstyle during the validate 
> phase, but
> we couldn't reach a consensus about it there:
> http://www.mail-archive.com/dev@commons.apache.org/msg58573.html
>
> On Fri, Aug 4, 2017 at 7:16 PM, Karl-Philipp Richter 
> <krichter@posteo.de>
> wrote:
>
>> Hi,
>> While working on a [small
>> contribution](https://issues.apache.org/jira/browse/MATH-1426) I 
>> noticed
>> that there's a checkstyle setup which is run in a reporting phase of
>> Maven which might be skipped by most developers and isn't used on 
>> Travis
>> CI.

Well, running
   $ mvn site
and checking the reports, is one of the (unwritten?) rule a contributor
to Commons will be told about. ;-)

>> I suggest to move this phase to the validate phase of Maven which
>> runs before the compile and test phase and in case of failure 
>> forbids
>> the invoker to build the project successfully.

Forcing style even before the compiler has the chance to warn about
invalid code looks a bit strong.

>> Therefore most
>> contributions will like they're intended too without the need of 
>> extra
>> communication.

If the above rule can be improved over, fine, but running CheckStyle
takes time (e.g. wrt to the compilation of a fix being worked on); so
it should be "mandatory" only before submitting a contribution.

Perhaps, we should have a maven profile "-P check-requirements"
which contributors _must_ run before providing a PR or attaching a
patch to JIRA.
That profile would thus contain your suggestion.

>>
>> The downside is that new (and eventually old) devs might be annoyed 
>> at
>> some point, especially if they frequently work on different projects
>> with different styles.

Indeed, hence my suggestion to not change the usual workflow but to
advertize that contribution will be taken into account only if they
pass the requirements.

>>
>> I can take over the move to the validate phase which is 10 lines
>> insertion/deletion in pom.xml, but not the definition of code style
>> rules which are common for the project because I don't know them. 
>> Doing
>> this change reveals about 400 issues of which > 95% are related to
>> missing or errornous Javadoc which is worth having a look at, but 
>> might
>> be postponed by deactivating the rule for now. Then you need to 
>> discuss
>> code style rules, because some, like the ones in the issue linked 
>> above,
>> aren't covered yet.

For "Commons Math", there is a custom "checkstyle.xml" (in the top
directory of the code repository).
When running "mvn site", CheckStyle currently reports 1 error.

The numerous errors you see (but which I do not) might be in the "test"
part of the source repository. [Historically, code style there was much
less emphasized (and perhaps checking it is disabled by in "mvn 
site").]

Best regards,
Gilles

>>
>> -Kalle Richter
>>
>>


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


Mime
View raw message