> On Apr 10, 2017, at 12:30 PM, Andrey Nazarov <andrey.x.naza...@oracle.com> 
> wrote:
> 
> Mandy, thanks for review. I’ve updated patch
> http://cr.openjdk.java.net/~anazarov/8178323/webrev.01/webrev/ 
> <http://cr.openjdk.java.net/~anazarov/8178323/webrev.01/webrev/>
> 

I suggest to add m4/p4.S to make it clear it’s a service type and m4 provides 
p4.S with  p4.Impl.
noOneUsesProvider will call —suggest-providers p4.S instead.

 239         static JLink run(boolean expectSuccess, String... options) {
Instead of adding a new parameter, what about adding a new 
runWithNonZeroExitCode method?

> See my comments inline
>> 
>> 
>> line 159-161: formatting nit: can you add spaces to align with the first
>> argument in line 158.  Same comment to SuggestProviders.java line 180-182
>> an 198-200.
> Fixed

line 216-219.

> Actually there are two issues here: 
> https://bugs.openjdk.java.net/browse/JDK-8178404 
> <https://bugs.openjdk.java.net/browse/JDK-8178404>
> https://bugs.openjdk.java.net/browse/JDK-8178405 
> <https://bugs.openjdk.java.net/browse/JDK-8178405>
Not really a bug but it’d be helpful to show that the provider exists but the 
service is not used in this case.
Mandy

Reply via email to