Categorizing javadoc errors

[Rémy Maucherat <re...@apache.org>]

Hi,

In the past, javadoc problems were fixed. I'm glad Mark switched
validation to checkstyle since I no longer trust the javadoc
developers for doing the right thing after the Java 18 changes. Also
it allows easy configuration of what is important and what should not
be fixed.

Looking at the list, I would propose:
- Remove javadoc validation for tests. This would mean doing
validation twice (once as usual, another one for the javadoc).
Although it is better to document everything, realistically we won't
be able to do everything.
- checkFirstSentence: After testing, this means that the javadoc first
(and quite often only) sentence should end with a period. This seems
pointless to me, so I plan to add a comment that this should be set to
"false".
- checkEmptyJavadoc: Actually this means the description is empty. It
happens often for certain obvious methods where @return is documented
instead. I'm slightly wavering on that one, in the generated HTML it
does indeed look better if both are set (to the same thing as the
content of @return - but with a period at the end, see the first
property).
- MissingJavadocMethod: This one is justified.
- MissingJavadocType: Totally justified.
- RequireEmptyLineBeforeBlockTagGroup: Should be ignored, another
pedantic arbitrary syntax rule.

Note: The error counts given include tests, so they're lower than that actually.

So: can I proceed with the separation between tests and non tests, and
then document why some settings will remain disabled ? This gives a
reasonable baseline where it can be assumed the rest is a useful
improvement.

Rémy

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

Re: Categorizing javadoc errors

[Mark Thomas <ma...@apache.org>]

On 17/01/2023 14:48, Rémy Maucherat wrote:
> Hi,
> 
> In the past, javadoc problems were fixed. I'm glad Mark switched
> validation to checkstyle since I no longer trust the javadoc
> developers for doing the right thing after the Java 18 changes. Also
> it allows easy configuration of what is important and what should not
> be fixed.
> 
> Looking at the list, I would propose:
> - Remove javadoc validation for tests. This would mean doing
> validation twice (once as usual, another one for the javadoc).
> Although it is better to document everything, realistically we won't
> be able to do everything.

Given we are going to have to split the validation, we might as well 
keep the existing Javadoc validation configuration for the test code so 
we maintain the standard we currently have.

> - checkFirstSentence: After testing, this means that the javadoc first
> (and quite often only) sentence should end with a period. This seems
> pointless to me, so I plan to add a comment that this should be set to
> "false".

I view that as a "nice to have" / cosmetic change. I might chip away at 
it slowly over time. If it ever got to the point where the test could 
pass then we could enable it at that point.

> - checkEmptyJavadoc: Actually this means the description is empty. It
> happens often for certain obvious methods where @return is documented
> instead. I'm slightly wavering on that one, in the generated HTML it
> does indeed look better if both are set (to the same thing as the
> content of @return - but with a period at the end, see the first
> property).

Another nice to have although given the improvement in the HTML output 
probably nicer to have than checkFirstSentence.

> - MissingJavadocMethod: This one is justified.
> - MissingJavadocType: Totally justified.

Agreed, but a lot of work to add. I expect it will take a long time to 
clear all those errors.

> - RequireEmptyLineBeforeBlockTagGroup: Should be ignored, another
> pedantic arbitrary syntax rule.

Is that what all of the Checkstyle rules are ;)
Another in the nice to have category for me but mainly because I like 
consistently formatted code as I find it easier to read.

> Note: The error counts given include tests, so they're lower than that actually.
> 
> So: can I proceed with the separation between tests and non tests, and
> then document why some settings will remain disabled ? This gives a
> reasonable baseline where it can be assumed the rest is a useful
> improvement.

+1

Can I suggest the disabled settings are described with something like:
"Disabled. Large number of errors with minimal benefit to be gained by 
fixing at this time."

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org