> 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