LGTM with nits.

http://gwt-code-reviews.appspot.com/1045801/diff/33001/34002
File
dev/core/src/com/google/gwt/core/ext/linker/ModuleMetricsArtifact.java
(right):

http://gwt-code-reviews.appspot.com/1045801/diff/33001/34002#newcode44
dev/core/src/com/google/gwt/core/ext/linker/ModuleMetricsArtifact.java:44:
this(SoycReportLinker.class, nextInstance.getAndAdd(1));
Nit: there's a "getAndIncrement()" for the +1 case.

http://gwt-code-reviews.appspot.com/1045801/diff/33001/34004
File dev/core/src/com/google/gwt/core/linker/SoycReportLinker.java
(right):

http://gwt-code-reviews.appspot.com/1045801/diff/33001/34004#newcode117
dev/core/src/com/google/gwt/core/linker/SoycReportLinker.java:117:
artifacts = new ArtifactSet(artifacts);
Nit, but the old formulation was okay because it conditionally created a
new artifact set only when there was something to update, relying on
returning the new artifact set.  If you would rather unconditionally
replace the old set (unlikely to be much of a perf hit), then all the
helpers can just return void and you return the original set at the
bottom; makes the data flow easier to eyeball.

http://gwt-code-reviews.appspot.com/1045801/diff/33001/34005
File dev/core/src/com/google/gwt/dev/Precompile.java (right):

http://gwt-code-reviews.appspot.com/1045801/diff/33001/34005#newcode619
dev/core/src/com/google/gwt/dev/Precompile.java:619:
precompilationMetrics.setAstTypes(unifiedAst.getReferencedJavaClasses());
See comment in JavaToJavaScriptCompiler.

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

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

Reply via email to