[gwt-contrib] Re: Move com.google.gwt.requestfactory to com.google.requestfactory (issue1383808)
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
[gwt-contrib] Re: Move com.google.gwt.requestfactory to com.google.requestfactory (issue1383808)
Hollow out the to-be-deleted classes to avoid the massive duplication of code going on in this patch. The old types should extend the new types and where that isn't possible, the old type should be rewritten as a delegate. Anything in the rebind or **/impl packages can be a straight move. They're not public types. http://gwt-code-reviews.appspot.com/1383808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Move com.google.gwt.requestfactory to com.google.requestfactory (issue1383808)
I spent a few days attempting this -- it's not so simple. For example: 1) Enums can't extend Enums. Several public types contain nested Enums. 2) If a type oldA extends newA, and a method in it returns an instance of oldA, you cant simply delegate to the implementation in newA since that will return a newA, which is not an instanceof oldA. So you need to write wrappers for everything. 3) It's unclear to me how inheritance works on annotations 4) Generators will have to be modified to work off the different annotation types I think there were other problems as well that I can't recall at the moment. My belief is that continuing with this approach would have polluted the new code base with special cases to handle parallel enums and annotations. I spoke to Ray about it and he agreed to the plan of leaving a snapshot in the old location and trying to remove it in the next release. On Thu, Mar 24, 2011 at 8:57 AM, b...@google.com wrote: Hollow out the to-be-deleted classes to avoid the massive duplication of code going on in this patch. The old types should extend the new types and where that isn't possible, the old type should be rewritten as a delegate. Anything in the rebind or **/impl packages can be a straight move. They're not public types. http://gwt-code-reviews.appspot.com/1383808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: Move com.google.gwt.requestfactory to com.google.requestfactory (issue1383808)
Yeah, Dan made the valiant effort here but I don't think it's practical. I don't feel bad asking users to change import statements to pick up bug fixes. It would be great if we can get this into the 2.3 rc. On Mar 24, 2011 7:05 AM, Daniel Rice (דניאל רייס) r...@google.com wrote: I spent a few days attempting this -- it's not so simple. For example: 1) Enums can't extend Enums. Several public types contain nested Enums. 2) If a type oldA extends newA, and a method in it returns an instance of oldA, you cant simply delegate to the implementation in newA since that will return a newA, which is not an instanceof oldA. So you need to write wrappers for everything. 3) It's unclear to me how inheritance works on annotations 4) Generators will have to be modified to work off the different annotation types I think there were other problems as well that I can't recall at the moment. My belief is that continuing with this approach would have polluted the new code base with special cases to handle parallel enums and annotations. I spoke to Ray about it and he agreed to the plan of leaving a snapshot in the old location and trying to remove it in the next release. On Thu, Mar 24, 2011 at 8:57 AM, b...@google.com wrote: Hollow out the to-be-deleted classes to avoid the massive duplication of code going on in this patch. The old types should extend the new types and where that isn't possible, the old type should be rewritten as a delegate. Anything in the rebind or **/impl packages can be a straight move. They're not public types. http://gwt-code-reviews.appspot.com/1383808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
