Brian Slesinsky has posted comments on this change.
Change subject: Implemented handling of GwtIncompatible annotations.
......................................................................
Patch Set 2:
(7 comments)
Seems good so far. I'm surprised it was this simple.
....................................................
File dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java
Line 235: private class ParserImpl extends Parser {
I think this can be a static class? JdtCompiler is getting kind of big, so
maybe move this class to top level instead of keeping it as an nested
class. Perhaps call it JdtParser?
Line 261: * Filters out types that have an annotation of class {@code
*.GwtIncompatible}; recursibly
"recursively"
Line 270: tyDecl.memberTypes =
stripGwtIncompatibleElements(tyDecl.memberTypes);
A possible simplification: if there is a loop in
stripGwtIncompatibleElements (see below) then we could move this line there
as well. That way stripGwtIncompatibleElements would be self-recursive and
there wouldn't be any mutual recursion.
Line 282: * Filters out methods that have an annotation of class
{@code *.GwtIncompatible}.
"Modifies the methods array of each type to remove any GwtIncompatible
methods."
Style nit: I'm not sure why this method should contain the outer loop?
Perhaps it could take one type as an argument, and the loop should be in
stripGwtIncompatibleElements. Similarly for stripGwtIncompatibleFields, and
then the two loops can be combined.
Line 305: * Filters out fields that have an annotation of class {@code
*.GwtIncompatible}.
"Modifies the fields array of each type to remove any GwtIncompatible
fields."
Line 333: private TypeDeclaration[]
stripGwtIncompatibleElements(TypeDeclaration[] types) {
It looks like this method and all the other methods it calls could be
static.
....................................................
File
user/test/com/google/gwt/dev/jjs/test/gwtincompatible/GwtIncompatible.java
Line 24: * The presence of this annotation on a method indicates that the
method may
We will need to explain how GwtIncompatible works somewhere that shows up
in GWT's external javadoc, and it won't here because this is test code. So
I think maybe we should put a copy of this class under
com.google.gwt.core.shared in the user package.
In this class we can say:
A GwtIncompatible annotation for use in a test.
@see com.google.gwt.core.shared.GwtIncompatible
--
To view, visit https://gwt-review.googlesource.com/2320
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f995d107b9e24fb7afb8aca95ab99b35f561216
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Roberto Lublinerman <[email protected]>
Gerrit-Reviewer: Brian Slesinsky <[email protected]>
Gerrit-Reviewer: Matthew Dempsky <[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.
