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