Mostly LGTM
http://gwt-code-reviews.appspot.com/1214801/diff/1/2 File user/src/com/google/gwt/jsonp/client/JsonpRequest.java (right): http://gwt-code-reviews.appspot.com/1214801/diff/1/2#newcode33 user/src/com/google/gwt/jsonp/client/JsonpRequest.java:33: Added whitespace. http://gwt-code-reviews.appspot.com/1214801/diff/1/2#newcode78 user/src/com/google/gwt/jsonp/client/JsonpRequest.java:78: Whitespace. http://gwt-code-reviews.appspot.com/1214801/diff/1/2#newcode129 user/src/com/google/gwt/jsonp/client/JsonpRequest.java:129: * Create a new JSONP request. Shouldn't this distinguish from the above constructor, explaining why this would be used? http://gwt-code-reviews.appspot.com/1214801/diff/1/2#newcode142 user/src/com/google/gwt/jsonp/client/JsonpRequest.java:142: String callbackParam, String failureCallbackParam, String id) { Need javadoc for id. http://gwt-code-reviews.appspot.com/1214801/diff/1/3 File user/src/com/google/gwt/jsonp/client/JsonpRequestBuilder.java (right): http://gwt-code-reviews.appspot.com/1214801/diff/1/3#newcode203 user/src/com/google/gwt/jsonp/client/JsonpRequestBuilder.java:203: failureCallbackParam, predeterminedId); Would it be cleaner to just pass predeterminedId to the constructor, and have existing behavior if it is null? That would also remove duplication in the constructors. http://gwt-code-reviews.appspot.com/1214801/diff/1/6 File user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java (right): http://gwt-code-reviews.appspot.com/1214801/diff/1/6#newcode67 user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java:67: Whitespace. http://gwt-code-reviews.appspot.com/1214801/diff/1/6#newcode91 user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java:91: Whitespace. http://gwt-code-reviews.appspot.com/1214801/diff/1/6#newcode154 user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java:154: Whitespace. Something must be set wrong in your IDE to generate all these where they didn't exist previously. http://gwt-code-reviews.appspot.com/1214801/diff/1/8 File user/test/com/google/gwt/resources/ExternalTextResourceJsonp.gwt.xml (right): http://gwt-code-reviews.appspot.com/1214801/diff/1/8#newcode16 user/test/com/google/gwt/resources/ExternalTextResourceJsonp.gwt.xml:16: <set-configuration-property name="ExternalTextResource.useJsonp" value="true" /> 80 columns http://gwt-code-reviews.appspot.com/1214801/diff/1/11 File user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java (right): http://gwt-code-reviews.appspot.com/1214801/diff/1/11#newcode26 user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java:26: static interface Resources extends ClientBundleWithLookup { Why package protected? http://gwt-code-reviews.appspot.com/1214801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
