> On Dec 19, 2016, at 10:26 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
>
> On 19/12/16 18:06, Mandy Chung wrote:
>> Updated webrev:
>>
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8171418/webrev.01
>
> Looks good to me.
> A related question is whether the tokens ALL-SYSTEM, ALL-DEFAULT,
> ALL-MODULE-PATH, should appear in the jdeps help message?
>
I’ll leave it as is. Clean it up when we do another pass on the help message.
Mandy
> best regards,
>
> -- daniel
>
>>
>>> On Dec 19, 2016, at 9:54 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
>>>
>>> Hi Mandy,
>>>
>>> DepsAnalyzer.java:
>>>
>>> 232 Collection<Module> modules =
>>> configuration.getModules().values();
>>>
>>> I don't see where modules is used in this method.
>>> Should this line be removed?
>>>
>>
>> Removed. Lance also caught that.
>>
>>>
>>> 163 if (filter.requiresFilter().isEmpty()) {
>>> 164 return archives.stream()
>>> 165 .filter(this::include)
>>> ...
>>> 168 } else {
>>> 169 // use the archives that have dependences and not specified
>>> in --require
>>> 170 return archives.stream()
>>> 171 .filter(source ->
>>> !filter.requiresFilter().contains(source.getName()))
>>> 172 .filter(this::include)
>>> ...
>>>
>>> and later:
>>>
>>> 266 public boolean include(Archive source) {
>>> 267 Module module = source.getModule();
>>> 268 // skip system module by default
>>> 269 return !module.isSystem()
>>> 270 || configuration.rootModules().contains(source)
>>> 271 || filter.requiresFilter().contains(module.name());
>>> 272 }
>>>
>>>
>>> If I look at how JdepsFilter::includes was implemented, and given that
>>> filter.requiresFilter().contains(module.name()) will always be false
>>> if we come from line 165, or from line 172 then I would suggest to
>>> remove line 271: this would ensure that the call at line 95 does
>>> the same than before.
>>>
>>
>> That’s a good suggestion.
>>
>> Mandy
>>
>>>
>>> On 19/12/16 05:42, Mandy Chung wrote:
>>>> Webrev at:
>>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8171418/webrev.00
>>>>
>>>> --include-system-modules option was a temporary option added in an early
>>>> stage of developement. Time to remove it as a clean up.
>>>>
>>>> Mandy
>>>>
>>>
>>
>
