> On Apr 8, 2016, at 7:52 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
> 
> Hi,
> 
> Please find below a patch that slightly modifies
> the JEP 264 initial implementation to take advantage
> of the module system.
> 
> The change modifies the LoggerFinder abstract service
> to take the Module of the caller class as parameter
> instead of the caller class itself.
> The caller class was used in the initial 9/dev implementation
> because java.lang.reflect.Module was not yet available.
> 
> http://cr.openjdk.java.net/~dfuchs/jigsaw/webrev_8148568/webrev.01/

Using the Module as the caller granularity is reasonable here to find the 
resource bundle.

DefaultLoggerFinder.java
 135         static boolean isSystem(Module m) {
 136             ClassLoader cl = AccessController.doPrivileged(new 
PrivilegedAction<ClassLoader>() {

This isSystem method should check if the class loader is platform class loader 
(ClassLoader::getPlatformClassLoader) or its ancestor. There are several copies 
of this method. Better to have one single copy of this method shared for other 
classes to use?

Nit: you can use the diamond operation PrivilegedAction<>.

LazyLoggers.java.
line 294                 return new LazyLoggerAccessor(name, factories, module);

I spot several long lines in LogManager.java, Logger.java, System.java and 
tests, maybe other files.  It’d be good if you can wrap the lines to help 
side-by-side review.

Other that that, looks good.

Mandy

Reply via email to