I dont mind the checkstyle stuff, but we have /not/ had a vote that mandates this to be a requirement. Until we do so I think we should safely just add "skip" to checkstyle where it's inappropriate, or ignore more stuff.
Kristian 2014-10-25 20:36 GMT+02:00 Andreas Gudian <andreas.gud...@gmail.com>: > I've started switching on maven-parent 25 in surefire now and I see > literary hundreds of checkstyle errors (I ignore the warnings for now). > Most of the stuff is fine with me and I'm on fixing them for a couple of > days already. > > But there's one thing that struck me and that I'm a bit reluctant to change > - and I'd like your opinion on it. > > We have for example this method: > private @Nonnull List<String> getExcluded() ... > > Checkstyle now tells me that the @Nonnull should preceed the private > modifier: > > @Nonnull private List<String> getExcluded() ... > > Especially as Java 8 introduced the notion of annotated type declarations > (as in @Nonnull String myNeverNullVariable; ), I think that this particular > @Nonnull annotation should be right in front of the return type and not > before the visibility modifier. > > The message is thrown by the ModifierOrder check (which does other good > things) and that one can't be configured to behave differently. > > So now I'd have to specifically ignore that warning on each method for > which we qualify the return type with @Nonnull / @Nullable. > > What's you pick on this? Is the form suggested by checkstyle really to be > preferred? I'd open an issue for them if you also think that it might need > some tweaking. > > Thanks, > Andreas > > > > 2014-10-14 8:34 GMT+02:00 Hervé BOUTEMY <herve.bout...@free.fr>: > >> Le mardi 14 octobre 2014 09:28:21 Olivier Lamy a écrit : >> > On 14 October 2014 05:01, Hervé BOUTEMY <herve.bout...@free.fr> wrote: >> > > 2. if we configured Checkstyle to report an error, this means check >> should >> > > fail: if you find that it should not fail, please help improve >> Checkstyle >> > > configuration by setting severity to warning only >> > >> > /me not a checkstyle expert configuration :-) >> this makes me remember there is a feature improvement for m-checkstyle-p >> 2.13 >> that I should explain: I was not a Checkstyle configuration master and felt >> like you >> Than I consistently reported Checkstyle rule name and category in every >> message and report, and a link to Checkstyle documentation for each rule >> [1] >> >> this gives a good intro to Checkstyle configuration and help a lot when >> needing >> to @SuppressWarnings( "checkstyle:name of rule, in lowercase") >> >> > /me asking himself if having such hard checkstyle requirement help to >> > improve user experience. >> from my perspective, previous feature really improved m-checkstyle-p >> experience >> and I know I should submit patches to Checkstyle itself, because I think >> this >> could help their users too when using bare-Checkstyle >> >> Regards, >> >> Hervé >> >> >> [1] >> http://maven.apache.org/plugins/maven-checkstyle-plugin/checkstyle.html >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org >> For additional commands, e-mail: dev-h...@maven.apache.org >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
