On 7/1/13 6:25 AM, Daniel Fuchs wrote:
Looks good in general. Some comments:
Logger.global is deprecated. In LogManager static initializer, we
should have @SuppressWarnings("deprecation") on Logger.global (probably
need to define a local variable) to suppress the warning and together
with your comment it will make it clear that accessing this deprecated
field is intentional.
Right. I can do that. But I don't see any warnings when compiling
(with make) - maybe they're turned out by default?
Deprecation and a few other javac warnings are disabled (see
jdk/makefiles/Setup.gmk).
As you pointed out, the same pattern is used in the checkPermission()
method. Might be worth having a private getManager() method to replace
direct access to the manager field in the Logger class - just a thought
to prevent similar bug from happening again.
Well - hmm - I'd prefer to do that in a later cleanup.
I'd want first to analyze all the places where manager is used.
Plus there is Peter's suggestion about changing the dynamics
between LogManager & Logger.
Both are excellent suggestions but I think I'd like to dedicate
a whole changeset for this kind of cleanup, without mixing it
with bug fix.
Agree. It's good to do it in a separate fix especially it may involve
untangling the Logger and LogManager initialization.
Mandy
