Hi Mandy & Peter,
Here is an update patch applying mandy's suggestions:
http://jmmc.fr/~bourgesl/share/webrev-2-8010309/
Good work and I like the solution.
>
Thanks.
> JavaLogger and LoggerProxy are only used from within PlatformLogger.
> Wouldn't it be better to declare them private? Their usage (one or the
> other) depends on the 'loggingSupport' flag that is initialized with
> PlatformLogger class. Usage of JavaLogger or LoggerProxy outside
> PlatformLogger would be wrong, I think.
>
>
> Yes JavaLogger and LoggerProxy classes can be made private.
>
Done.
I looked at it but not in great detail. In general it's very good clean
> up. The comment in line 124-132 is good information and since the code is
> evolving, I would suggest to take those count out.
>
Fixed.
> valueOf is one common method name to find an instance of a given value.
> Perhaps LevelEnum.forValue can be renamed to LevelEnum.valueOf(int)?
>
Done.
> It seems to be useful to add a static method LevelEnum.getLevel(int) to
> replace the calls to "LevelEnum.forValue(newLevel).julLevel".
>
I agree I prefer using methods (field encapsulation) so I added:
+ /**
+ * java.util.logging.Level optionally initialized in
JavaLogger's static initializer
+ * and USED ONLY IN JavaLogger (only once java.util.logging
is available and enabled)
+ */
+ private Object julLevel;
+
+ void setJulLevel(Object julLevel) {
+ this.julLevel = julLevel;
+ }
+
+ Object getJulLevel() {
+ return this.julLevel;
+ }
+
+ /**
+ * Return the java.util.logging.Level used only in JavaLogger
+ * @param levelValue PlatformLogger level as int
+ * @return java.util.logging.Level or null if
java.util.logging is not available and disabled
+ */
+ static Object getJulLevel(int levelValue) {
+ return valueOf(levelValue).getJulLevel();
+ }
The tests are in jdk/test/java/util/logging and jdk/test/sun/util/logging.
> It'd be good to check if you want to extend
> jdk/test/sun/util/logging/PlatformLoggerTest.java to cover all levels (it's
> currently already checking several).
>
I will now have a look ...
Regards,
Laurent
