maven-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dennis Lundberg <denn...@apache.org>
Subject Re: errors reported by Checkstyle
Date Sun, 26 Oct 2014 09:19:06 GMT
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" <
kristian.rosenvold@gmail.com>:

> 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.gudian@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.boutemy@free.fr>:
> >
> >> Le mardi 14 octobre 2014 09:28:21 Olivier Lamy a écrit :
> >> > On 14 October 2014 05:01, Hervé BOUTEMY <herve.boutemy@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-unsubscribe@maven.apache.org
> >> For additional commands, e-mail: dev-help@maven.apache.org
> >>
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message