Hi Alan,
Thank you for the review!
I'll implement all your suggestions.
Thanks,
Serguei
On 6/21/16 04:56, Alan Bateman wrote:
On 21/06/2016 11:48, serguei.spit...@oracle.com wrote:
Sorry.
My initial plan was to write an nsk.jvmti test and review it on a
confidential mailing list.
It is why I put the webrevs on the non-public server.
Forgot to switch the server when the test was converted into the
jtreg format.
The public webrevs are:
Hotspot:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/
Jdk:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/
The spec generally looks good, just two small points:
1. "java.lang.ClassLoader" would be better than
"java/lang/ClassLoader" when describing the class_loader parameter.
2. For the package name then it might be clearer to say that the
package name is in internal form and then include the example of
"java/lang" in parenthesis.
One comment on the test is that it looks like check_system_loader
doesn't test any packages that are defined to a module.
check_bootstrap_loader does have tests for named modules for the null
loader case. Just wondering if we need more here.
You might have spotted this already but the header comment on the
tests needs to be the GPL header.
-Alan
