Mostly LGTM, some comments.

http://gwt-code-reviews.appspot.com/33804/diff/24/30
File
dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java
(right):

http://gwt-code-reviews.appspot.com/33804/diff/24/30#newcode388
Line 388: * @param modifyPrimaryJavascript(strongName) strong name of
the module being emitted
???

http://gwt-code-reviews.appspot.com/33804/diff/24/31
File
dev/core/src/com/google/gwt/core/ext/linker/impl/StandardStatementRanges.java
(right):

http://gwt-code-reviews.appspot.com/33804/diff/24/31#newcode27
Line 27: private static int[] toArray(List<Integer> list) {
Danger Will Robinson: if the passed-in list is a LinkedList, this will
be N^2.  I'd either use enhanced for, or else toArray() -> Integer[]
then manually unbox.

http://gwt-code-reviews.appspot.com/33804/diff/24/33
File dev/core/src/com/google/gwt/core/linker/IFrameLinker.java (right):

http://gwt-code-reviews.appspot.com/33804/diff/24/33#newcode64
Line 64: if (charsPerChunk < 0) {
Would it improve anything to short-circuit:

  || js.length() < charsPerChunk?

http://gwt-code-reviews.appspot.com/33804/diff/24/33#newcode68
Line 68: StringBuffer sb = new StringBuffer();
This should be a StringBuilder rather than a StringBuffer.

http://gwt-code-reviews.appspot.com/33804/diff/24/43
File user/src/com/google/gwt/core/CompilerParameters.gwt.xml (right):

http://gwt-code-reviews.appspot.com/33804/diff/24/43#newcode19
Line 19: This is the maximum number of variables in any var statement
GWT will emit. This avoids a bug in
Looks like this was reformatted from 80 cols to something longer?

http://gwt-code-reviews.appspot.com/33804/diff/24/44
File user/super/com/google/gwt/emul/java/lang/Class.java (right):

http://gwt-code-reviews.appspot.com/33804/diff/24/44#newcode123
Line 123: + (seedName != null ? seedName : "" + clazz.hashCode());
Unrelated change, please commit to trunk by itself; LGTM.

http://gwt-code-reviews.appspot.com/33804/diff/24/42
File user/test/com/google/gwt/emultest/EmulSuite.gwt.xml (right):

http://gwt-code-reviews.appspot.com/33804/diff/24/42#newcode25
Line 25: <set-configuration-property
name='iframe.linker.script.chunk.size' value='100'/>
Is EmulSuite the right place for these?  Seems like something part of
CompilerSuite would be more appropriate; this is for JRE tests and
stuff.

http://gwt-code-reviews.appspot.com/33804

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

Reply via email to