On Fri, 18 Oct 2024 08:39:49 GMT, Nizar Benalla <[email protected]> wrote:
>> Please review this change that adds a new test root `docs` dedicated to
>> testing the documentation, which has been a work in progress for a while.
>> Tests for links, encoding, HTML, accessibility will be later added in
>> following PRs.
>>
>> We also define a new make target `test-docs` meant for local use and depends
>> on the docs.
>> This also adds the necessary configurations needed at Oracle.
>>
>> This patch includes a test `TestDocs` which serves to show developers how
>> they are meant to resolve the docs to test them, I want to include it
>> temporarily until better tests are added later.
>>
>> TIA
>
> Nizar Benalla has updated the pull request incrementally with one additional
> commit since the last revision:
>
> use existing DOCS_JDK_IMAGE_DIR to be consistent.
>
> no ifneq check before assigning the env variable because
> DOCS_JDK_IMAGE_DIR is set even docs do not exist. It will be up to the test
> to check if the files are there
make/conf/jib-profiles.js line 1008:
> 1006:
> testOnlyProfilesPrebuiltDocs["run-test-prebuilt-docs"].dependencies.push("docs.doc_api_spec",
> "tidy");
> 1007:
> testOnlyProfilesPrebuiltDocs["run-test-prebuilt-docs"].environment["DOCS_JDK_IMAGE_DIR"]
> = input.get("docs.doc_api_spec", "install_path");
> 1008:
> testOnlyProfilesPrebuiltDocs["run-test-prebuilt-docs"].environment["TIDY"] =
> input.get("tidy", "home_path") + "/bin/tidy";
If we are doing another round here anyway, could you try to break up these
lines a bit? I'm not enforcing strict 80 chars in the js file, but this is
wider than a comfortable side-by-side diff is able to render for me.
Suggestion:
testOnlyProfilesPrebuiltDocs["run-test-prebuilt-docs"].environment["DOCS_JDK_IMAGE_DIR"]
= input.get("docs.doc_api_spec", "install_path");
testOnlyProfilesPrebuiltDocs["run-test-prebuilt-docs"].environment["TIDY"]
= input.get("tidy", "home_path") + "/bin/tidy";
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21272#discussion_r1806408094
