I agree, and believe that it was Hervè's intention for this to be a "soft" rule, at least to start with.
-- Dennis Lundberg Den 26 okt 2014 07:47 skrev "Kristian Rosenvold" < [email protected]>: > 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 <[email protected]>: > > 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 <[email protected]>: > > > >> Le mardi 14 octobre 2014 09:28:21 Olivier Lamy a écrit : > >> > On 14 October 2014 05:01, Hervé BOUTEMY <[email protected]> > 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: [email protected] > >> For additional commands, e-mail: [email protected] > >> > >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
