Looks good.
+1 -- note that needs a second reviewer (doesn't need to be a capital-R
Reviewer).
-- Kevin
On 7/3/2018 8:56 AM, Kevin Rushforth wrote:
PS: I am not really satisfied by adding such noise in build.gradle,
but it can be improved later ...
Agreed. This can be a follow-on issue. I'll finish my review shortly.
-- Kevin
On 7/3/2018 8:45 AM, Laurent Bourgès wrote:
Kevin,
> I added the system property "ClipShapeTest.numTests" but it
requires a
build.gradle change to pass the parameter:
Yes, something like this is what I had in mind. As long as we
don't add too many of these, it is OK with me. Note that as coded,
the build will fail if you don't define ClipShapeTest.numTests, so
you will need to check for that. I note also that you used tabs in
build.gradle (so please change them to spaces). I recommend the
following logic:
if (rootProject.hasProperty("ClipShapeTest.numTests")) {
systemProperty "ClipShapeTest.numTests",
rootProject.getProperty("ClipShapeTest.numTests")
}
I adopted your proposal and updated the webrev:
http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-092.2/
<http://cr.openjdk.java.net/%7Elbourges/marlinFX/marlinFX-092.2/>
PS: I am not really satisfied by adding such noise in build.gradle,
but it can be improved later ...
Laurent
