LGTM.  One very minor code change suggested, and several comment changes
suggested.


http://gwt-code-reviews.appspot.com/65805/diff/1/4
File user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
(right):

http://gwt-code-reviews.appspot.com/65805/diff/1/4#newcode1403
Line 1403:
checksum.update(Util.getBytes(type.getQualifiedSourceName()));
If I read the code correctly, this checksum is now recomputed every time
computeClassPrefix is called.  It used to be computed once per instance
of the generator.  That's a little unfortunate, but I guess it's a
little too much work to bother about it right now.

It might be worth a todo to use a checksum of the CSS Contents rather
than a checksum of a typeoracle snapshot.  That would seem to give very
stable compiles as well as eliminate this algorithmic complexity
problem.

http://gwt-code-reviews.appspot.com/65805/diff/1/4#newcode1572
Line 1572: * impossible for other generators to produce CssResource
interfaces.
It's worth a TODO here, too, to clarify the intent.  It might be that
the concern is no longer an issue.  I just talked to Scott, and nowadays
the spec is that generators run in the same order if identical source
code is recompiled.  That order can change due to minute changes in the
source code, but not if the source code is bit for bit the same.

http://gwt-code-reviews.appspot.com/65805/diff/1/14
File user/src/com/google/gwt/uibinder/rebind/MortalLogger.java (right):

http://gwt-code-reviews.appspot.com/65805/diff/1/14#newcode24
Line 24: * Treelogger.
For what it's worth, putting them in TreeLogger sounds good to me.

http://gwt-code-reviews.appspot.com/65805/diff/1/23
File
user/src/com/google/gwt/uibinder/rebind/model/ImplicitCssResource.java
(right):

http://gwt-code-reviews.appspot.com/65805/diff/1/23#newcode60
Line 60: */
Auto-generated comment.

http://gwt-code-reviews.appspot.com/65805/diff/1/23#newcode86
Line 86: urls[i++] = found.nextElement();
If you just want one (which is logical), what about
classLoader.getResource()  (singular) ?

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

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

Reply via email to