Review request for jdeps option rename to Java-style and enhancements to output dot file format
Paul Sandoz
paul.sandoz at oracle.com
Mon Oct 14 15:59:14 UTC 2013
On Oct 13, 2013, at 10:31 PM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> On 11/10/2013 08:08, Mandy Chung wrote:
>> There were some discussion of jdeps CLI to follow GNU style option
>> or the traditional Java style. As there will be plans to upgrade
>> the JDK tools to GNU style options, it'd be desirable to keep jdeps
>> be consistent with other langtools and be upgraded to GNU style in
>> a unified manner.
>>
>> This patch includes the fixes for:
>> JDK-8015912: jdeps output in dot graph format and option to find API dependences
>> JDK-8026255: Switch jdeps to follow traditional Java option style
>>
>> Webrev at:
>> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8015912/webrev.00/
> Command line options are somewhat subjective, I think what you have is reasonable. I thought the GNU style that were in the original version were okay too (just takes getting used to) but keeping them consistent with similarly named options in the other tools is important tool. One change that I didn't quite understand is dropping -R for recursive, that one seemed intuitive (to me).
>
> Anyway, on the changes then I went through the webrev and I didn't see anything obviously wrong. A few comments:
>
> - In PlatformClassPath then alt-rt is special-cased and I think that can be removed now.
>
> - Also in PlatformClassPath is special handling of jfxrt.jar (if it is present) isn't clear. I recall this came up before and maybe it just needs a more detailed comment to explain why it is needed.
>
> - In Analyzer then I think I missed why the results are a LinkedHashMap, wouldn't HashMap do?
>
It preserves order of corresponding to the order of archives passed in, but AFAICT that is only taken into account when iterating on line 85, and such an order does not seem to be relevant.
Paul.
> - A minor comment in JdepsTask but I assume the OutputWriter interface can be private
>
> -Alan
More information about the core-libs-dev
mailing list