The current changes can be found here: http://cr.openjdk.java.net/~reinhapa/reviews/8161230/webrev.03
On 07.09.2016 02:39, Mandy Chung wrote: > There was already an discussion about this with Alan Bateman: >> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-August/042792.html >> > I sample a few existing APIs and they document UncheckedIOException in the > method description rather than @throws as Alan described. > >> So I do not know is the best solution for this… > Your method description already covers it. It’s fine. line 1386-1387 in > @apiNote is redundant that I still think can be taken out. Those lines are gone now ;-) >>> I have reservation in adding ClassLoader::systemResources static method. >>> >>> ClassLoader::getSystemResources is a convenient method that is equivalent >>> to calling ClassLoader::getSystemClassLoader().getResources(), since the >>> system class loader is never null. This method includes the resources in >>> the application’s classpath. >>> >>> With the new ClassLoader::getPlatformClassLoader() method, a better way to >>> get the “system” resources from JDK (not the ones from the classpath), it >>> can call: >>> ClassLoader::getPlatformClassLoader().resources(); >>> >>> To get a Stream of URL of the resources visible to system class loader, it >>> can call the new resources method: >>> ClassLoader::getSystemClassLoader().resources(); >>> >>> So it doesn’t seem that the proposed systemResources static method is >>> necessary. >>> >> Hmm... We may have overlooked this, when we started by just looking into the >> API changes in general. I did those with Paul Sandoz in more deep in the >> following thread: >> >> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-August/042752.html >> >> and this proposal is based on the "go" from Paul here: >> >> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-August/043012.html > I file a bug to update the spec ClassLoader::getSystemClassLoader to return > non-null: > https://bugs.openjdk.java.net/browse/JDK-8165563 > >>> I suggest to combine the two tests in a single test, ResourcesStreamTest >>> and add @bug JBS-id >>> >>> ResourcesSuccessCase >>> >>> 46 List<URL> resources = stream.collect(Collectors.toList()); >>> 52 URL url = resources.get(0); >>> >>> You can check the size first. This can be simplified to use >>> filter/findFirst to file the expected URL (yes need to call cl.resources >>> again that is not an issue). >>> >> I have done this also in the new revision > testFailure method can be simplified to > long count = cl.resources(“the name”).count(); > if (count != 1) > throw new Exception("expected resource is null or empty"); > > cl.resources("the name”) > .filter(url -> "file:/somefile".equals(url.toExternalForm()) > .findFirst() > .orElseThrow(…) > > Mandy I also made those changes as you suggested.... -Patrick
