Re: [Hawkular-dev] Checkstyle javadoc settings

From: "Jay Shaughnessy" <jshaughn(a)redhat.com&gt; To: hawkular-dev(a)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/res... >> >> >> -- 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(a)redhat.com&gt;: >>>>>> >>>>>> 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-commo... >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Regards >>>>>>>>>> Gary >>>>>>>>>> _______________________________________________ >>>>>>>>>> hawkular-dev mailing list >>>>>>>>>> hawkular-dev(a)lists.jboss.org >>>>>>>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev >>>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> hawkular-dev mailing list >>>>>>>>> hawkular-dev(a)lists.jboss.org >>>>>>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev >>>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> hawkular-dev mailing list >>>>>>>> hawkular-dev(a)lists.jboss.org >>>>>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev >>>>>>>> >>>>>>> >>>>>> _______________________________________________ >>>>>> hawkular-dev mailing list >>>>>> hawkular-dev(a)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(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev >>>>> >>>> _______________________________________________ >>>> hawkular-dev mailing list >>>> hawkular-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev >>>> >>> _______________________________________________ >>> hawkular-dev mailing list >>> hawkular-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/hawkular-dev >>> > _______________________________________________ > hawkular-dev mailing list > hawkular-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hawkular-dev _______________________________________________ hawkular-dev mailing list hawkular-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hawkular-dev