+1 fantastic! ~ David Smiley Apache Lucene/Solr Search Developer http://www.linkedin.com/in/davidwsmiley
On Mon, Nov 4, 2019 at 10:45 AM Andrzej Białecki <[email protected]> wrote: > +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 <[email protected]> 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: [email protected] > > For additional commands, e-mail: [email protected] > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
