[Hawkular-dev] Checkstyle javadoc settings

Stefan Negrea snegrea at redhat.com
Wed Feb 4 10:26:16 EST 2015 

This morning, I noticed a lot of activity on this mailing list and got really excited; only to be flabbergasted that the vast majority is about checkstyle. I would rather have a very popular project than a project with pristine code by whatever standards but absolutely no users. With the time put so far into this, we are getting dangerously close to the "bike-shed effect", not to mentioned that any overly bureaucratic process has the side effect of slowing down real progress ...

Thank you,
Stefan 

----- Original Message -----
> From: "Jay Shaughnessy" <jshaughn at redhat.com>
> To: hawkular-dev at lists.jboss.org
> Sent: Wednesday, February 4, 2015 8:24:08 AM
> Subject: Re: [Hawkular-dev] Checkstyle javadoc settings
> 
> 
> +1 to Thomas.  If a method has jdoc then it should be correct. If it
> doesn't then no problem.  I think at the class level it should always
> have some kind of jdoc. And no formatting of jdoc, which is a change I
> believe we already made to the formatter imports.
> 
> On 2/4/2015 8:30 AM, Thomas Segismont wrote:
> > Le 04/02/2015 13:49, Peter Palaga a écrit :
> >> On 02/04/2015 10:57 AM, Thomas Segismont wrote:
> >>> There's also "allowMissingJavadoc". I'm +1 for adding Javadoc checks
> >>> provided we don't fail the build on missing Javadoc.
> >> Not failing and just having the warnings in the build log is useless.
> > There's a misunderstanding: I'm talking about not failing the build if a
> > single method does not have Javadoc.
> >
> > If a method has Javadoc which does not represent reality (wrong list of
> > parameters for example) then yes, the build should fail.
> >
> >> What is not enforced, will be ignored.
> > That's not my opinion.
> >
> >> I am against introducing anything that can be ignored or overseen to
> >> checkstyle.
> > See my first comment, I'm talking about not failing the build if a
> > single method does not have Javadoc.
> >
> >>> It's not just
> >>> getters/setters: sometimes your method has a meaningful name and it does
> >>> not make any sense to write Javadoc for it.
> >> I do not believe that something like this can be configured with
> >> Checkstyle.
> > Isn't that the purpose of "allowMissingJavadoc"?
> >
> >> However given that the general idea of adding JavaDoc checks has become
> >> some level of support here, Gary or anybody else, feel free to submit a
> >> PR to our checkstyle.xml [1] plus do not forget to test your proposal
> >> with metrics, bus and alerts.
> >>
> >> [1]
> >> https://github.com/hawkular/hawkular-build-tools/blob/master/src/main/resources/hawkular-checkstyle/checkstyle.xml
> >>
> >>
> >> -- P
> >>
> >>> Le 04/02/2015 10:02, Gary Brown a écrit :
> >>>> Hi
> >>>>
> >>>> According to the docs
> >>>> (http://checkstyle.sourceforge.net/config_javadoc.html#JavadocMethod)
> >>>> the 'allowMissingPropertyJavadocs' parameter can be used to skip
> >>>> checking of getters/setters.
> >>>>
> >>>> Regards
> >>>> Gary
> >>>>
> >>>> ----- Original Message -----
> >>>>> Hey,
> >>>>>
> >>>>> I am in favor of JavaDoc and having some checking - especially as in
> >>>>> RHQ we
> >>>>> have many places
> >>>>> where the param list of a method and the one in JavaDoc have
> >>>>> diverged over
> >>>>> time.
> >>>>>
> >>>>> I am not in favor of forcing javadoc on every method (especially
> >>>>> getter/setter), as this will just end up in
> >>>>>
> >>>>> /** This is the getter for foo */
> >>>>> public foo getFoo() {}
> >>>>>
> >>>>> Which is imo worse than no doc.
> >>>>>
> >>>>> Unfortunately I think the default for JavaDoc is not to document
> >>>>> private
> >>>>> properties (=not include in the generated html),
> >>>>> so that putting the comment on the property itself does not help for
> >>>>> browsing
> >>>>> docs.
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>> Am 04.02.2015 um 09:34 schrieb Gary Brown <gbrown at redhat.com>:
> >>>>>>
> >>>>>> Hi Peter
> >>>>>>
> >>>>>> The main reason I mentioned it was because although I had been
> >>>>>> diligent in
> >>>>>> Overlord re javadoc, once the rule was enabled it picked up many
> >>>>>> issues -
> >>>>>> primarily inconsistency between parameter names, or missing parameter
> >>>>>> entries.
> >>>>>>
> >>>>>> I agree meaningful text can only be picked up by review, but think
> >>>>>> that
> >>>>>> areas where automated checking is possible shouldn't be part of the
> >>>>>> reviewers responsibility (i.e. to reduce their burden so they can
> >>>>>> focus on
> >>>>>> other areas).
> >>>>>>
> >>>>>> Regards
> >>>>>> Gary
> >>>>>>
> >>>>>> ----- Original Message -----
> >>>>>>>> Not sure if this was previously discussed and decided
> >>>>>>> Not that I knew. We decided to start from a very minimal set of rules
> >>>>>>> that we initially copied from wildfly. We have not changed much:
> >>>>>>> we just
> >>>>>>> increased the line length to 120 chars and extended the plaintext
> >>>>>>> checks
> >>>>>>> to non-java files.
> >>>>>>>
> >>>>>>> I am personally undecided about JavaDoc checks. Having a meaningful
> >>>>>>> JavaDoc is a good thing.
> >>>>>>> Checkstyle can certainly help to some extent, but:
> >>>>>>> (1) It is not enough as it will never check the meaningfulness
> >>>>>>> (2) I tend to believe that some methods (incl. getters and
> >>>>>>> setters) do
> >>>>>>> not need JavaDoc
> >>>>>>> (3) Non-public methods often should have JavaDoc too.
> >>>>>>>
> >>>>>>> I am a strong proponent of four eyes principle: no single commit
> >>>>>>> can go
> >>>>>>> to master without being reviewed properly. It should be reviewer's
> >>>>>>> responsibility to check test coverage, JavaDoc, etc.
> >>>>>>>
> >>>>>>> Best wishes,
> >>>>>>>
> >>>>>>> -- Peter
> >>>>>>>
> >>>>>>> On 02/03/2015 07:19 PM, Gary Brown wrote:
> >>>>>>>> It checks presence of javadoc, and matching entries for
> >>>>>>>> parameters and
> >>>>>>>> return values.
> >>>>>>>>
> >>>>>>>> ----- Original Message -----
> >>>>>>>>> Does this just look to see if all public methods have SOME javadoc?
> >>>>>>>>> (i.e.
> >>>>>>>>> it
> >>>>>>>>> just sees if they are missing)
> >>>>>>>>>
> >>>>>>>>> Does it impose some type of formatting as well?
> >>>>>>>>>
> >>>>>>>>> ----- Original Message -----
> >>>>>>>>>> Hi
> >>>>>>>>>>
> >>>>>>>>>> Just started using the hawkular parent pom and noticed that the
> >>>>>>>>>> checkstyle
> >>>>>>>>>> config does not check the javadoc comments.
> >>>>>>>>>>
> >>>>>>>>>> Not sure if this was previously discussed and decided that it
> >>>>>>>>>> shouldn't
> >>>>>>>>>> be
> >>>>>>>>>> checked, but thought I had better check, as this is one area
> >>>>>>>>>> that can
> >>>>>>>>>> be
> >>>>>>>>>> time consuming to update code after enabling.
> >>>>>>>>>>
> >>>>>>>>>> Previously I had been using this config in Overlord:
> >>>>>>>>>> https://github.com/Governance/overlord-commons/blob/master/overlord-commons-build/src/main/resources/checkstyle/checkstyle.xml#L83-L97
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Regards
> >>>>>>>>>> Gary
> >>>>>>>>>> _______________________________________________
> >>>>>>>>>> hawkular-dev mailing list
> >>>>>>>>>> hawkular-dev at lists.jboss.org
> >>>>>>>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
> >>>>>>>>>>
> >>>>>>>>> _______________________________________________
> >>>>>>>>> hawkular-dev mailing list
> >>>>>>>>> hawkular-dev at lists.jboss.org
> >>>>>>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
> >>>>>>>>>
> >>>>>>>> _______________________________________________
> >>>>>>>> hawkular-dev mailing list
> >>>>>>>> hawkular-dev at lists.jboss.org
> >>>>>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
> >>>>>>>>
> >>>>>>>
> >>>>>> _______________________________________________
> >>>>>> hawkular-dev mailing list
> >>>>>> hawkular-dev at lists.jboss.org
> >>>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
> >>>>> --
> >>>>> Reg. Adresse: Red Hat GmbH, Technopark II, Haus C,
> >>>>> Werner-von-Siemens-Ring 14, D-85630 Grasbrunn
> >>>>> Handelsregister: Amtsgericht München HRB 153243
> >>>>> Geschäftsführer: Charles Cachera, Michael Cunningham, Paul Hickey,
> >>>>> Charlie
> >>>>> Peters
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> hawkular-dev mailing list
> >>>>> hawkular-dev at lists.jboss.org
> >>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
> >>>>>
> >>>> _______________________________________________
> >>>> hawkular-dev mailing list
> >>>> hawkular-dev at lists.jboss.org
> >>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
> >>>>
> >>> _______________________________________________
> >>> hawkular-dev mailing list
> >>> hawkular-dev at lists.jboss.org
> >>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
> >>>
> > _______________________________________________
> > hawkular-dev mailing list
> > hawkular-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/hawkular-dev
> 
> _______________________________________________
> hawkular-dev mailing list
> hawkular-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hawkular-dev
>

More information about the hawkular-dev mailing list