On Jun 21, 2012, at 2:44 PM, Alan Bateman wrote: > On 21/06/2012 08:55, Joe Wang wrote: >> : >> webrev: >> http://cr.openjdk.java.net/~joehw/jdk8/7169894/webrev/ > It's great to get this work started. > > The javadoc changes look okay to me and fine for this change. However there > are a few areas that could be made clearer, like specifying the behavior for > the case that is error encountered locating the providers. Also it does not > specific which class loader is used. > > On the implementation changes then I agree with Paul's comment that there is > a lot of duplication. I realize you have inherited some technical debt but > now seems a good opportunity to clean this up instead of changing essentially > the same code in lots of places. > > I don't understand the need for the Class.forName in findServiceProvider as I > thought this method should just use ServiceLoader. >
Good catch, one would expect the service interfaces to be visible. > I see Paul also suggested that the default/fallback implementation not be > registered but an important need here is that the JAXP module shouldn't need > to include all of Xerces and Xalan. I think it would be cleaner to use > services mechanism rather than using an optional dependency. > Ah! I incorrectly thought a default service provider class would always be present. Paul.
