Goktug Gokdogan has posted comments on this change.
Change subject: Changes StackTraceDeobfuscator to abstract and provide
various factory methods.
......................................................................
Patch Set 1:
(8 comments)
....................................................
File user/src/com/google/gwt/core/server/impl/StackTraceDeobfuscator.java
Line 54: * {@link ClassLoader}.
Done
Line 59: return new StackTraceDeobfuscator() {
I don't think this will be a real issue, as if somebody really needs to
know which anonymous class it is, it will be easy to follow by the line
number.
Line 72: * Creates a deobfuscator that loads symbol map files from the
given file path.
Done.
About parameter type: As in most google3 scenarios, directory name is
usually available as string, so this is more convenient for developer.
Line 83: * Creates a deobfuscator that loads symbol map files from the
given url path.
Done. (javadoc)
URL doesn't mean HTTP protocol. A file or resource from a jar can all be
identified via URL. All other fromXXX methods above could have written by
calling this method instead of anonymous class (I am just doing a minor
optimization by not using this).
For example; JunitHost will be using this one to get the files from the
servlet context..
Line 85: public static StackTraceDeobfuscator fromFileSystem(final URL
urlPath) {
Opps. Thanks for catching that.
Line 98: private static class SymbolCache {
I also like them at the end if they are private. Especially, in this case
keeping the public static methods on the top helps with discoverability.
Google java style guide is more helpful on this as it only asks for logical
order. Anyway I am OK to change it if this is a strict rule or this class
is going to be of the few exceptions..
Line 158: * Note that, this has no effect on permutations that the
symbols have already been loaded for.
Done.
There are 3 logical states here; loaded / partially loaded / not loaded.
Applied that to your suggestion.
Line 338: protected abstract InputStream openInputStream(String fileName)
throws IOException;
Added more description on the javadoc.
Currently this method is also called repeatedly which usually doesn't make
sense if it is a "not found' error.
I like none of that, however I have other priorities right now. We can
defer it as the problem was always there.
--
To view, visit https://gwt-review.googlesource.com/2270
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I562e052caef8da7f3434319cf11b8984bc347fe5
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <[email protected]>
Gerrit-Reviewer: Brian Slesinsky <[email protected]>
Gerrit-Reviewer: Goktug Gokdogan <[email protected]>
Gerrit-HasComments: Yes
--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "Google Web Toolkit Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
