[Hawkular-dev] Checkstyle javadoc settings

Gary Brown gbrown at redhat.com
Wed Feb 4 10:29:28 EST 2015


Hi Stefan

I don't disagree with your point - the only reason I raised it was that it is one of those rules, that if not sorted out upfront, can be very expensive to fix up later on.

Regards
Gary

----- Original Message -----
> 
> 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
> > 
> 
> _______________________________________________
> 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