LGTM Talked offline. Agree that your change is pretty safe. But please be sure to land the corresponding addon-gwt change in lock step.
On Mon, Sep 27, 2010 at 2:49 PM, Ray Ryan <[email protected]> wrote: > Let's hold off on this one until we find a real solution to the bad create > api issue. The sketch in https://jira.springsource.org/browse/ROO-1456will > not do it. > > > On Mon, Sep 27, 2010 at 12:13 PM, Ray Ryan <[email protected]> wrote: > >> >> >> On Mon, Sep 27, 2010 at 12:10 PM, <[email protected]> wrote: >> >>> >>> http://gwt-code-reviews.appspot.com/926801/diff/1/12 >>> File >>> >>> user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java >>> (right): >>> >>> http://gwt-code-reviews.appspot.com/926801/diff/1/12#newcode241 >>> >>> user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:241: >>> if (publicProxyType.equals(entityProxyType)) { >>> Yes, it is to cover the case of Request<EntityProxy>. It was previously >>> used when the find method did not have a generic signature. There were >>> tests for this stuff at that point, but there are currently no tests >>> exercising this use case. >>> >>> Okay to add one in a subsequent patch? >> >> >> No. If it is no longer needed, please delete it. If it is still need it, >> please test it. >> >>> >>> >>> On 2010/09/27 18:33:50, rjrjr wrote: >>> >>>> Is this case Request<EntityProxy> someMethod()? >>>> >>> >>> If so, is that case actually covered in the tests? >>>> >>> >>> http://gwt-code-reviews.appspot.com/926801/diff/1/13 >>> File >>> >>> user/src/com/google/gwt/requestfactory/server/ReflectionBasedOperationRegistry.java >>> (right): >>> >>> http://gwt-code-reviews.appspot.com/926801/diff/1/13#newcode116 >>> >>> user/src/com/google/gwt/requestfactory/server/ReflectionBasedOperationRegistry.java:116: >>> public boolean isReturnTypeSet() { >>> It seems to be just a simple, convenience method. It is used in exactly >>> one place in the source, and that too, to print a helpful error method. >>> Will inline this method at its call sites. >> >> >> Only if you can do the same to isRerturnTypeList, please >> >>> >>> >>> On 2010/09/27 18:33:50, rjrjr wrote: >>> >>>> It's surprising that this method hasn't been missed until now. Are >>>> isReturnTypeList and isReturnTypeSet actually called from anywhere? If >>>> >>> not, >>> >>>> shouldn't we delete them? >>>> >>> >>> @RayC? >>>> >>> >>> http://gwt-code-reviews.appspot.com/926801/show >>> >> >> > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
