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
<krich...@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-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
