Hi Serguei,

I added the explanation as a comment in parseOptions what is longOptsMap,
and how parseOptions() work in new webrev.

  http://cr.openjdk.java.net/~ysuenaga/JDK-8226204/webrev.01/

I'm not good at English, so comments are welcome :)


Thanks,

Yasumasa


On 2019/08/15 17:12, [email protected] wrote:
Hi Yasumasa,

In fact, I have a problem to understand what the parseOptions() method is doing.
Could you add necessary comments explaining what is done in the loop?
I'm sure I'll be not alone in having trouble to read this code.
Also, it is not clear the approach with the longOptsMap's.
Why do you need to map "exe=" to "exe" but "mixed" to "-m" and"clstats" to 
"-clstats"?
It is better to be explained in the parseOptions() method as well.

Thanks,
Serguei


On 8/10/19 04:14, Yasumasa Suenaga wrote:
PING: Could you review it?

   JBS: https://bugs.openjdk.java.net/browse/JDK-8226204
   webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8226204/webrev.00/


Yasumasa


On 2019/07/24 10:18, Yasumasa Suenaga wrote:
Hi all,

Please review this change:

   JBS: https://bugs.openjdk.java.net/browse/JDK-8226204
   webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8226204/webrev.00/

This enhancement has been proposed in [1].

SALauncher (jhsdb implementation) processes the option for each subcommand 
(e.g. jstack, hsdb).
But they exist in many place with similar code.
So there is some room for refactoring.

This change has passed the tests on submit repo and serviceability/sa tests.


Thanks,

Yasumasa


[1] 
https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-June/028376.html

Reply via email to