LGTM with nits.

http://gwt-code-reviews.appspot.com/1380806/diff/29005/user/src/com/google/gwt/resources/client/ImageResource.java
File user/src/com/google/gwt/resources/client/ImageResource.java
(right):

http://gwt-code-reviews.appspot.com/1380806/diff/29005/user/src/com/google/gwt/resources/client/ImageResource.java#newcode127
user/src/com/google/gwt/resources/client/ImageResource.java:127: String
getURL();
Should this be deprecated?

http://gwt-code-reviews.appspot.com/1380806/diff/29005/user/src/com/google/gwt/resources/rg/DataResourceGenerator.java
File user/src/com/google/gwt/resources/rg/DataResourceGenerator.java
(right):

http://gwt-code-reviews.appspot.com/1380806/diff/29005/user/src/com/google/gwt/resources/rg/DataResourceGenerator.java#newcode68
user/src/com/google/gwt/resources/rg/DataResourceGenerator.java:68:
sw.println("new " + DataResourcePrototype.class.getName() + "(");
Should this be getCanonicalName()?  In this case they are the same, but
in general getName() is not going to return a name you can use in the
source.

If you change, change them all in this change.

http://gwt-code-reviews.appspot.com/1380806/diff/29005/user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
File user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
(right):

http://gwt-code-reviews.appspot.com/1380806/diff/29005/user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java#newcode481
user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java:481:
new String[]{bundle.getNormalContentsFieldName(),
bundle.getRtlContentsFieldName()};
space between ] and {

http://gwt-code-reviews.appspot.com/1380806/diff/29005/user/src/com/google/gwt/user/client/ui/Image.java
File user/src/com/google/gwt/user/client/ui/Image.java (right):

http://gwt-code-reviews.appspot.com/1380806/diff/29005/user/src/com/google/gwt/user/client/ui/Image.java#newcode301
user/src/com/google/gwt/user/client/ui/Image.java:301: public abstract
void setVisibleRect(Image image, int left, int top, int width, int
height);
On 2011/04/27 17:18:58, xtof wrote:
Maybe undo this line wrap?

Since we changed to 100 chars, my understanding is each file when edited
should be reformatted, though ideally as a separate change to avoid
obfuscating the real change.  In this case, I don't care much either
way.

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

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

Reply via email to