Thumbs up!

Thanks,
David

On 25/02/2015 2:17 AM, Kumar Srinivasan wrote:
Hi David,

comments inlined.

Hi Kumar,

On 24/02/2015 8:14 AM, Kumar Srinivasan wrote:
Hello,

Please review the fix for the above issue.

Webrev:
http://cr.openjdk.java.net/~ksrini/8066185/webrev.00/

The fix is self explanatory, as for the test I have done the following:

I found the comment:

          /*
+          * Since this is a VM flag, we need to ensure it is not an
+          * application argument, meaning the argument must precede
+          * the main class or those flags that invoke the VM directly.
+          */

a bit confusing - specifically the "or those flags that invoke the VM
directly". Took me while to realize that all the args you treat
specially (-version, -h, -jar etc) are all "terminal" arguments -
either the launcher stops looking at args after the given arg, or all
following args must be for the application, not the launcher or VM. I
would have expressed this as:

  /*
   * Since this must be a VM flag we stop processing once we see
   * an argument the launcher would not have processed beyond (such
   * as -version or -h), or an argument that indicates the following
   * arguments are for the application (i.e. the main class name, or
   * the -jar argument).
   */


+1,  I have updated the webrev with your comments, also had an AHA moment,
I realized, had missed -X, which prints the XUsage, added this to the logic
and test.

a. refactored it from a single longish test to sub-tests for
readability.
b. added new sub-test for NMT Argument Processing checks.

Can you please update the @summary for that test. It seems the OSX
specific test was hijacked for NMT arg testing and the summary was
never updated to reflect that.

This test was "commandeered" :-) for other purposes, updated the summary to
describe what is being performed.

Webrev to last reviewed:
http://cr.openjdk.java.net/~ksrini/8066185/webrev.01/webrev.delta/index.html


Full webrev:
http://cr.openjdk.java.net/~ksrini/8066185/webrev.01/index.html

Thanks.
Kumar


Thanks,
David

Thanks
Kumar


Reply via email to