Review request for jdeps option rename to Java-style and enhancements to output dot file format
Mandy Chung
mandy.chung at oracle.com
Tue Oct 15 03:38:11 UTC 2013
Alan,
Thanks for the review. Here is the updated webrev per your feedback:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8015912/webrev.01/
I have also fixed
jdk/test/sun/reflect/CallerSensitive/CallerSensitiveFinder.java due to
the change of ClassFileReader.newInstance method.
More comments inlined below.
On 10/13/2013 1:31 PM, Alan Bateman wrote:
> 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).
>
Initially I considered that -recursive is a less frequently used option
as typically we would be interested in the immediate dependencies. I
also think that -R is an intuitive short option to -recursive and there
shouldn't be an issue when it's upgraded to gnu-style in the future and
so I add it back.
> 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.
>
That's right (thanks for reminding it). I have removed it.
> - 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.
>
PlatformClassPath.contains method seems to be a bit confusing. I added
a new JDKArchive class that will be instantiated for JDK jar file
> - In Analyzer then I think I missed why the results are a
> LinkedHashMap, wouldn't HashMap do?
>
It doesn't need to be ordered and I have changed it to HashMap. That was
a left-over change I made in an earlier version of this fix. JdepsTask
already maintains an ordered list of archives (sourceLocations).
> - A minor comment in JdepsTask but I assume the OutputWriter interface
> can be private
>
OutputWriter was in an earlier version but was removed. It's not in
webrev.00 and perhaps you were looking at an earlier version?
Mandy
More information about the core-libs-dev
mailing list