> On Apr 8, 2016, at 7:52 AM, Daniel Fuchs <[email protected]> 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