LGTM.

Since you have already taken care of the "merge error", there are only
minor nits below. Plus, the comments I sent you earlier. Please submit
after these changes.

One other thing we discussed was to add a RequestFactoryTestSuite class.
If you'd like, you can just add a TODO for now.


http://gwt-code-reviews.appspot.com/700802/diff/10001/11032
File
user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java
(left):

http://gwt-code-reviews.appspot.com/700802/diff/10001/11032#oldcode186
user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java:186:
assert violations != null;
Change this to a Map<Long, String> instead of using "toString()". Same
at other places.

Again, if you want to defer it to another CL, that is fine. Just put a
TODO in here.

http://gwt-code-reviews.appspot.com/700802/diff/10001/11034
File
user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java
(left):

http://gwt-code-reviews.appspot.com/700802/diff/10001/11034#oldcode510
user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:510:

Add a TODO to merge these methods as well.

http://gwt-code-reviews.appspot.com/700802/diff/10001/11035
File
user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
(right):

http://gwt-code-reviews.appspot.com/700802/diff/10001/11035#newcode55
user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:55:

Use JSONObject instead of String here? Maybe add a TODO.

http://gwt-code-reviews.appspot.com/700802/diff/10001/11044
File user/test/com/google/gwt/i18n/client/resolutiontest/Inners.java
(right):

http://gwt-code-reviews.appspot.com/700802/diff/10001/11044#newcode40
user/test/com/google/gwt/i18n/client/resolutiontest/Inners.java:40:
Revert changes to this file?

http://gwt-code-reviews.appspot.com/700802/diff/10001/11045
File user/test/com/google/gwt/requestfactory/RequestFactoryTest.gwt.xml
(right):

http://gwt-code-reviews.appspot.com/700802/diff/10001/11045#newcode22
user/test/com/google/gwt/requestfactory/RequestFactoryTest.gwt.xml:22:

32ec6eff339a4c66e6e7179b9d5a17d846804069:google3/third_party/java_src/gwt/svn/trunk/user/test/com/google/gwt/requestfactory/RequestFactoryTest.gwt.xml
Merge error?

http://gwt-code-reviews.appspot.com/700802/diff/10001/11049
File user/test/com/google/gwt/user/client/WindowTest.java (right):

http://gwt-code-reviews.appspot.com/700802/diff/10001/11049#newcode70
user/test/com/google/gwt/user/client/WindowTest.java:70:
revert changes to this file?

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

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

Reply via email to