On Mon, 23 Sep 2024 23:03:54 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
> disable the compileTestJava task in the :swt project if IS_TEST_JAVAFX_SDK
> is true.
disabled the task accordingly.
Thanks for the review, updated the PR as per comments.
The names TEST_SDK_PATH and IS_TEST_SDK seem better.
> build.gradle line 733:
>
>> 731:
>> 732: ext.IS_TEST_JAVAFX_SDK = false
>> 733: if (hasProperty("TEST_JAVAFX_SDK_PATH")) {
>
> Suggestion: drop `JAVAFX_` from the name of this property and just call it
> `TEST_SDK_PATH`. That's closer to what we did in JDK 8 when we had a similar
> feature (it was called `TEST_SDK`, but I think adding `_PATH` as a suffix is
> good).
Changed name as `TEST_SDK_PATH`
> build.gradle line 745:
>
>> 743: fail("The provided TEST_JAVAFX_SDK_PATH=${TEST_JAVAFX_SDK_PATH}
>> is invalid")
>> 744: }
>> 745: ext.IS_TEST_JAVAFX_SDK = true
>
> Similarly, you might consider removing `JAVAFX_` from this property, but it's
> an internal-only variable, so it doesn't matter.
Changed name as `IS_TEST_SDK`
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1577#issuecomment-2370292396
PR Review Comment: https://git.openjdk.org/jfx/pull/1577#discussion_r1772696103
PR Review Comment: https://git.openjdk.org/jfx/pull/1577#discussion_r1772696056
