Please fix DTRF and re-enable the commented out block before submitting.
http://gwt-code-reviews.appspot.com/929801/diff/1/2 File user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java (right): http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode157 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:157: public void diagnosticIf(boolean cond, String fmt, Object... args) should not be public. http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode192 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:192: allowedRequestParamTypes.add(JPrimitiveType.CHAR); Do we have tests of the primitive param types? http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode266 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:266: private void diagnosticIfBannedType(JType origType, String entityName, String name) name > methodName http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode275 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:275: private void diagnosticIfBannedType(Set<JType> allowed, JType origType, ditto http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode285 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:285: diagnosticIf(setType.isAssignableFrom(origType.isClassOrInterface()) Why not use cType here? http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode288 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:288: && !listType.getErasedType().equals(cType.getErasedType()), The set and list checks could be factored into another method rather than being copy pasted like this, matchesErasedType(setType, cType) http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode292 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:292: && !entityProxyType.isAssignableFrom(origType.isClassOrInterface()) use cType http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode303 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:303: if (setType.isAssignableFrom(typeArg) Looks like you meant to say listType in one of these. Again, please refactor and reuse rather than copying and pasting, even for short things like this. http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode339 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:339: } catch (DiagnosticException e) { Why not have a single catch around generateOnce? If you're worried about signature pollution make it a runtime execption http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode487 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:487: private List<JMethod> findMethods(JClassType domainType, String name) { More descriptive names on the two findMethods, please, since they seem to find different things. Or make a single method, and make the method collection its input rather than baking in the particular call? http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode645 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:645: try { Move to call to generateOnce http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode775 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:775: continue; Why is this needed? Please document. If it's speculative, please remove. http://gwt-code-reviews.appspot.com/929801/diff/1/2#newcode1329 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:1329: "Proxy %s is missing @ProxyFor annotation", entityName); Redundant check for this in maybeDecode, right? http://gwt-code-reviews.appspot.com/929801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
