[XERCESJ-1781] fine tuning Javadoc linting#65
[XERCESJ-1781] fine tuning Javadoc linting#65SingingBush wants to merge 2 commits intoapache:mainfrom
Conversation
| /** | ||
| * Sample entry. | ||
| * <div>Usage: <KBD>org.apache.xerces.utils.regex.REUtil <regex> <string></KBD></div> | ||
| * <p>Usage: <code>org.apache.xerces.utils.regex.REUtil <regex> <string></code></p> |
There was a problem hiding this comment.
should probably be samp instead of code if I recall correctly
There was a problem hiding this comment.
Can't use samp in Javadoc due to:
error: unknown tag: samp
could use pre if preferred
| additionalparam='-quiet ${additional.param}' | ||
| /> | ||
| failonerror='yes' | ||
| failonwarning='yes' |
There was a problem hiding this comment.
No. I do not want the build to fail on a warning. I'm not sure it should fail on an error here, but definitely not on a warning
There was a problem hiding this comment.
Why not? It helps having a cleaner codebase imho
There was a problem hiding this comment.
I've only set yes on the sections that don't have any errors or warnings. This branch passes, so setting yes here is to ensure future changes don't introduce problems.
There was a problem hiding this comment.
Saying that, I just saw that behaviour is different on the CI build. I'll take a look at why it's passing locally but not on CI
There was a problem hiding this comment.
I've been using JDK 8 locally, I'll make sure to fix errors for newer JDK linter problems as well.
There was a problem hiding this comment.
I was simply that later JDKs also error if table in a package.html file don't have a caption. Fixed now
| additionalparam='-quiet ${additional.param}' | ||
| /> | ||
| failonerror='yes' | ||
| failonwarning='yes' |
| additionalparam='-quiet ${additional.param}' | ||
| /> | ||
| failonerror='yes' | ||
| failonwarning='yes' |
|
please run CI on the most recent commit. Should be passing now |
|
This PR is a hard no. A warning is a warning, not a build failure We've already seen cases where javadoc warnings were simply incorrect; e.g. required captions on tables. And I triply don't want to encourage people to fill in javadoc with placeholder text just to make the build pass. |
|
If you like, you can send the javadoc fixes without the build.xml changes but I'm not going to fail the build on a warning. |
|
created new pr: #68 |
3 participants
Seeing light at the end of this tunnel. In this PR I've made some tweaks to the Ant build so that linting is targeting the right areas. With the PR the way it is the build log is down to 15 javadoc errors and 2 warnings. The build allows it to pass in this state. I still plan to fix the remaining javadoc errors (html based) in the code base. The linter is not set to check for missing. Perhaps that can be done in the future but for now I'd like to get to a point where XERCESJ-1781 can be considered done.