Review Request: JDK-8173374: Update GenGraphs tool to generate dot graph with requires transitive edges

[Daniel Fuchs]

Hi Mandy,

 > Updated webrev:
 >   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/

Looks good. I haven't reviewed the build changes.
I assume they're OK if you managed to build ;-)

best regards,

-- daniel

On 15/02/17 19:32, Mandy Chung wrote:
>
>> On Feb 15, 2017, at 10:29 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
>>
>> Hi Mandy,
>>
>> Some early comments:
>>
>> GenGraphs.java
>> --------------
>>
>>  58                 dir = Paths.get(args[++i]);
>>
>> may produced ArrayOutOfBoundsException - should we have better
>> error reporting?
>> Or should it check && i < args.length - 1 so that it falls back
>> to having dir == null below?
>>
>
> Good catch.  Fixed to:
>
>   i++;
>   dir = i < args.length ? Paths.get(args[i]) : null;
>
>
>>  93 .resolve(ModuleFinder.ofSystem(),
>>
>> could that be: .resolve(finder,
>>
>
> Fixed.
>
>>
>> Graph.java
>> ----------
>>
>> 119         return builder.build().reduce();
>> 277             this.nodes.addAll(nodes);
>>
>>
>> These were bugs, which you're taking this opportunity to fix - right?
>>
>
> Yes. 119 is caught by this change.  277 is caught by code inspection.
>
>>
>> JdepsTask.java:
>> ---------------
>>
>> 1027                 // print module descriptor
>>
>> Is this comment accurate?
>>
>
> I updated the comment:
>
> // generate dot graph from the resolved graph from module
> // resolution.  No class dependency analysis is performed.
>
>> DotFileTest.java
>> ----------------
>>
>> Missing @bug tag?
>>
>
> Fixed.
>
>
> Updated webrev:
>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173374/webrev.01/
>
> Mandy
>