8337111: Bad HTML checker for generated documentation #20711
Conversation
Add doccheck to tier 2
|
👋 Welcome back nbenalla! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@nizarbenalla The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
openjdk
bot
added
javadoc
nizarbenalla
changed the title
Webrevs
|
|
/issue add 8337113 8337114 8337116 |
nizarbenalla
changed the title
|
@nizarbenalla Adding additional issue to issue list: Adding additional issue to issue list: |
|
Sorry for the title change. |
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.nio.file.Path; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.concurrent.ExecutionException; | ||
| import java.util.concurrent.ExecutorService; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.Future; |
There was a problem hiding this comment.
It is often common practice to put Java SE and JDK imports first.
|
|
||
| @Before | ||
| public void setUp() { | ||
| Path root = Path.of(ROOT_PATH.getParent() + File.separator + "docs" + File.separator + System.getProperty("doccheck.dir")); |
There was a problem hiding this comment.
somewhat preferable to use Path.resolve instead of adding strings like this
| TidyChecker tidy = new TidyChecker(); | ||
| BadCharacterChecker badChars = new BadCharacterChecker(); | ||
| HtmlFileChecker docChecker = new HtmlFileChecker(new DocTypeChecker()); | ||
| HtmlFileChecker htmlChecker = new HtmlFileChecker(new LinkChecker()); |
There was a problem hiding this comment.
I suggest extracting these declarations into a method called something like getCheckers()
and then having two alternative methods to call them, called homelike like runSerially(List<Checker> checkers) and runWithExecutor(List<Checker> checkers)
- Renamed tools directory to doccheckutils due to naming conflict - Some fixes based on review comments
|
I tried to address some of the review comments, I now use |
|
Closing this for now, I need time to adapt parts of this |
Can I please get a review for this PR that adds 4 new html "Checkers" for the generated documentation.
More details are in the JBS issues
These tests were mostly inspired /converted from the existing Doccheck.
Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/20711/head:pull/20711$ git checkout pull/20711Update a local copy of the PR:
$ git checkout pull/20711$ git pull https://git.openjdk.org/jdk.git pull/20711/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 20711View PR using the GUI difftool:
$ git pr show -t 20711Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/20711.diff
Webrev
Link to Webrev Comment