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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
