The move looks pretty straightforward.  Just some thoughts that occurred
to me while looking at this:

  - RequestFactoryMagic should be moved out of the testing sub-package.
  - Are we happy with the client, shared, server package naming scheme?
  - Should the GWT-specific code in the new packages, like
RequestFactoryEditorDriver, be in a gwt subpackage like
com.google.requestfactory.gwt?
  - If there are more Android-specific changes that need to be made, it
might be worthwhile collecting those methods in a utility class.

These are all things that can be fined-tuned later, since doing them in
this CL would complicate the branch history unnecessarily.


http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml
File build.xml (right):

http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml#newcode196
build.xml:196: <!-- Build a jar file containing a subset of
requestfactory -->
In general, it seems like these targets should be in their own
requestfactory/build.xml along the lines of servlet/build.xml.

http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml#newcode226
build.xml:226: <requestfactory-jar target="client-src"/>
client+src

http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml#newcode252
build.xml:252: </classpath>
It should be possible to move these classpath elements into the first
macrodef without changing anything, since the test-only code isn't
reachable from the client seed classes.

http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml#newcode259
build.xml:259: <target name="requestfactory-jre-test"
depends="requestfactory-test+src" description="Run
RequestFactoryJreSuite">
This target should be added to the main "test" target.

http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/gwt/autobean/server/impl/BeanMethod.java
File user/src/com/google/gwt/autobean/server/impl/BeanMethod.java
(right):

http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/gwt/autobean/server/impl/BeanMethod.java#newcode208
user/src/com/google/gwt/autobean/server/impl/BeanMethod.java:208: //
since java.beans.Introspector is not available in Android 2.2
Switch to javadoc comment.

http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java
File user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java
(right):

http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java#newcode65
user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java:65: //
since that method is not available in Android 2.2
Javadoc comment.

http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/requestfactory/server/RequestFactoryJarExtractor.java
File
user/src/com/google/requestfactory/server/RequestFactoryJarExtractor.java
(right):

http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/requestfactory/server/RequestFactoryJarExtractor.java#newcode466
user/src/com/google/requestfactory/server/RequestFactoryJarExtractor.java:466:
for (Type t : Type.getArgumentTypes(desc)) {
This can be
  numArgs += t.getSize()
instead.

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

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

Reply via email to