ServiceLoader.load* take 2

[Alan Bateman]

On 15/06/2012 16:23, Paul Sandoz wrote:
> :
>
>
>> One thing I didn't quite get was the reason for moving LazyIterator1 out of ServiceLoader to a new org.openjdk.jigsaw.ModuleServiceLoader. My comment is really just wondering about the iteration/instantiation in two places rather than one.
>>
> I wanted to separate concerns so that ServiceLoader did not know any specifics of modular services (and vice versa FWIW). It should hopefully keep any future changes located in the org.openjdk.jigsaw package. I forgot to do so but it means, for example, we can make Loader.findServices protected or package private.
I see Mandy also commented on this too. Personally I thought it was 
marginally better to have the lazy loading iterators in one place rather 
than two but it does require that org.openjdk.jigsaw export a means to 
get the map without SL getting too tied into the implementation details. 
Part of the comment is probably the name too as MSL does not extend SL.


> :
>> On the new test then it reminds me that we may be able to consolidate the tests in the services directory into a more complete set of unit tests (just thinking that there is a bit of duplication between the new test the existing hello.sh, many.sh and optional.sh).
>>
> What do you propose?
>
> They definitely need to be fleshed out. Personally i prefer things to be in exploded form, i find it easier to grok.
>
When I added these tests then the idea was that they be standalone and 
easy to understand. I guess I'm just concerned that if they are 
copied/edited that they will come back to bite us as any change will 
require a change to every one of them. So just something to thing about, 
not important for this patch.

In any case, I'm happy to push these changes for you.

One other thing for ServiceLoader, a future round, is that the 
spec/javadoc will need to be updated to describe how it works with modules.

-Alan.