Updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8171418/webrev.01
> 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 >> >