+1, I think it’s an excellent idea. The check should also verify that the comment not only exists but also that it’s not empty - eg. there’s an IntelliJ template that creates an empty top-level javadoc.
> On 4 Nov 2019, at 16:40, Bram Van Dam <bram.van...@intix.eu> wrote: > > David Smiley mentioned this in the "SolrCloud is sick" thread. Instead > of hijacking that, I figured I'd start another thread. > > On 03/11/2019 05:32, David Smiley wrote: >> <snip> requiring javadocs on all top level classes. I think more javadocs >> and >> code comments would be very helpful -- especially for the major >> classes. > > This sounds like something that's actionable. > > I'm not sure if there are any guidelines regarding documentation on the > Solr project, but on my team there's a rule that says all classes must > have a top-level javadoc that explains the "why" of the class. "Why does > it exist/what's it for?" > > Excluding contrib, solrj and tests, there are some 400 source files with > classes with missing top level Javadoc. This includes some files with > undocumented nested "public static" classes -- couldn't find an obvious > way to exclude those using checkstyle. > > Here's a "top ten most frequently modified files with missing Javadoc" > below. This is an arbitrary metric, the "most referenced classes" might > be more useful, but that was harder to hack together with shell foo. > > solr/core/src/java/org/apache/solr/core/CoreContainer.java > solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java > solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java > solr/core/src/java/org/apache/solr/handler/StreamHandler.java > solr/core/src/java/org/apache/solr/cloud/ElectionContext.java > solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java > solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java > solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java > solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java > solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java > > If there's any interest in this, I could write a patch to include > something like this in the build (ant or gradle, whatever). > > - Bram > > Following checkstyle configuration detects classes with missing Javadoc: > > check.xml: > ========== > > <!DOCTYPE module PUBLIC > "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN" > "https://checkstyle.org/dtds/configuration_1_3.dtd";> > > <module name="Checker"> > <module name="TreeWalker"> > <module name="MissingJavadocType"/> > </module> > </module> > > Bit of shell foo to list offending files: > ========================================= > > java -jar checkstyle-8.26-all.jar -c config.xml solr/ | cut -d ' ' -f 2 > | sed "s:.*/lucene-solr/::g" | cut -d ':' -f 1 | sort | uniq > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
