LGTM

http://gwt-code-reviews.appspot.com/725801/diff/12001/13006
File
dev/core/src/com/google/gwt/dev/shell/rewrite/UseMirroredClasses.java
(right):

http://gwt-code-reviews.appspot.com/725801/diff/12001/13006#newcode43
dev/core/src/com/google/gwt/dev/shell/rewrite/UseMirroredClasses.java:43:
{
Make this is a static initialization block

static {
 ...
}

http://gwt-code-reviews.appspot.com/725801/diff/12001/13006#newcode48
dev/core/src/com/google/gwt/dev/shell/rewrite/UseMirroredClasses.java:48:
"com/google/gwt/logging/impl/DevModeLoggingFixes.getName");
For compile time safety & IDE refactoring, you could get the
class+method name from, say:
DevModeLoggingFixes.class.getName().replace(".", "/") + ".getName"

http://gwt-code-reviews.appspot.com/725801/diff/12001/13012
File user/src/com/google/gwt/logging/client/LogConfiguration.java
(right):

http://gwt-code-reviews.appspot.com/725801/diff/12001/13012#newcode63
user/src/com/google/gwt/logging/client/LogConfiguration.java:63:
root.setUseParentHandlers(false);
A quick comments here as to why we need to set this to false would be
good

http://gwt-code-reviews.appspot.com/725801/diff/12001/13013
File user/src/com/google/gwt/logging/impl/DevModeLoggingFixes.java
(right):

http://gwt-code-reviews.appspot.com/725801/diff/12001/13013#newcode25
user/src/com/google/gwt/logging/impl/DevModeLoggingFixes.java:25: * This
class is used by the byte code rewriting which happens in DevMode only.
Add link to the rewriting class for reference, or an @see javadoc if you
prefer

tiny nit, smaller than a nit below: "prefix which" -> "prefix, which"

http://gwt-code-reviews.appspot.com/725801/diff/12001/13013#newcode36
user/src/com/google/gwt/logging/impl/DevModeLoggingFixes.java:36: *
Replaces all logger.getName() calls
Also mention in the javadoc what the rewrite does.

http://gwt-code-reviews.appspot.com/725801/diff/12001/13013#newcode39
user/src/com/google/gwt/logging/impl/DevModeLoggingFixes.java:39: return
logger.getName().replace(GWT.getUniqueThreadId() + ".", "");
delegate to removeLoggerPrefix()

http://gwt-code-reviews.appspot.com/725801/diff/12001/13013#newcode73
user/src/com/google/gwt/logging/impl/DevModeLoggingFixes.java:73: return
name.replace(getLoggerPrefix(), "");
We could potentially replace additional occurrences of the prefix in the
user's logger name, although admittedly unlikely.

You could use a regex with a '^' anchor, or a simple substring() based
on length of the prefix.

http://gwt-code-reviews.appspot.com/725801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to