Matthew Dempsky has posted comments on this change.

Change subject: Ensure clinits get called for JSO instance methods.
......................................................................


Patch Set 2:

(3 comments)

....................................................
File user/test/com/google/gwt/dev/jjs/test/JsoTest.java
Line 49: static final class ClinitStaticFieldFirst extends JavaScriptObject { You have three identical class definitions here. I suspect that's because you have multiple test scenarios, and clinits aren't undoable. It would be nice to have a comment explaining that, so people don't come later on and think they can refactor the code and eliminate the extra tests. :)


Line 54:       if (FIELD == null) {
What's the point of all these 'if' statements? It looks like you should be able to eliminate them.

If they're necessary, can you please add a comment explaining why?


Line 84: static final class ClinitStaticMethodFirst extends JavaScriptObject {
Nit: Blank line before new class definition.


--
To view, visit https://gwt-review.googlesource.com/3361
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If09885382bcf2d6c149cd8e48bfdd3ae527d67cb
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Colin Alworth <[email protected]>
Gerrit-Reviewer: Colin Alworth <[email protected]>
Gerrit-Reviewer: John A. Tamplin <[email protected]>
Gerrit-Reviewer: Leeroy Jenkins <[email protected]>
Gerrit-Reviewer: Matthew Dempsky <[email protected]>
Gerrit-Reviewer: Roberto Lublinerman <[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 "GWT 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.


Reply via email to