You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Bram Van Dam <br...@intix.eu> on 2019/11/04 15:40:07 UTC
Missing top level javadocs
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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: Missing top level javadocs
Posted by David Smiley <da...@gmail.com>.
+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 <ab...@getopt.org> 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 <br...@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-unsubscribe@lucene.apache.org
> > For additional commands, e-mail: dev-help@lucene.apache.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>
Re: Missing top level javadocs
Posted by Andrzej Białecki <ab...@getopt.org>.
+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 <br...@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-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org