On 19/12/16 19:03, Mandy Chung wrote:
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.
OK. Looks good then!
-- daniel
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
