[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-08-03 Thread bobv 

LGTM, just a couple of code style nits I saw re-reading the patch.


http://gwt-code-reviews.appspot.com/714801/diff/48001/6003
File user/src/com/google/gwt/resources/client/DataResource.java (right):

http://gwt-code-reviews.appspot.com/714801/diff/48001/6003#newcode35
user/src/com/google/gwt/resources/client/DataResource.java:35: *
Specifies the resource or resources associated with the
Specifies that the

http://gwt-code-reviews.appspot.com/714801/diff/48001/6012
File
user/test/com/google/gwt/resources/client/DataResourceDoNotEmbedTest.java
(right):

http://gwt-code-reviews.appspot.com/714801/diff/48001/6012#newcode22
user/test/com/google/gwt/resources/client/DataResourceDoNotEmbedTest.java:22:
* Tests for code@link DataResource.DoNotEmbed @DoNotEmbed}/code
resource
Check the formatting of that @link.

http://gwt-code-reviews.appspot.com/714801/diff/48001/6012#newcode67
user/test/com/google/gwt/resources/client/DataResourceDoNotEmbedTest.java:67:
// Skip this test for browsers which do not support data URLs
return; here since it's an early out.

Remove the else block.  Apply in the other test class.

http://gwt-code-reviews.appspot.com/714801/diff/48001/6013
File
user/test/com/google/gwt/resources/client/DataResourceMimeTypeTest.java
(right):

http://gwt-code-reviews.appspot.com/714801/diff/48001/6013#newcode22
user/test/com/google/gwt/resources/client/DataResourceMimeTypeTest.java:22:
* Tests for code@link DataResource.MimeType @MimeType}/code resource
Weird formatting with the @link here.

http://gwt-code-reviews.appspot.com/714801/diff/48001/6013#newcode27
user/test/com/google/gwt/resources/client/DataResourceMimeTypeTest.java:27:
static interface Resources extends ClientBundle {
interfaces are implicitly static

http://gwt-code-reviews.appspot.com/714801/diff/48001/6013#newcode34
user/test/com/google/gwt/resources/client/DataResourceMimeTypeTest.java:34:
static final String FOUR_ZEROS_SOURCE = fourZeros.dat;
Remove the static final modifiers, since that's implicit.

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-08-03 Thread fredsa 

Thanks, Bob


http://gwt-code-reviews.appspot.com/714801/diff/48001/6003
File user/src/com/google/gwt/resources/client/DataResource.java (right):

http://gwt-code-reviews.appspot.com/714801/diff/48001/6003#newcode35
user/src/com/google/gwt/resources/client/DataResource.java:35: *
Specifies the resource or resources associated with the
On 2010/08/03 13:15:30, bobv wrote:

Specifies that the


Done.

http://gwt-code-reviews.appspot.com/714801/diff/48001/6012
File
user/test/com/google/gwt/resources/client/DataResourceDoNotEmbedTest.java
(right):

http://gwt-code-reviews.appspot.com/714801/diff/48001/6012#newcode22
user/test/com/google/gwt/resources/client/DataResourceDoNotEmbedTest.java:22:
* Tests for code@link DataResource.DoNotEmbed @DoNotEmbed}/code
resource
On 2010/08/03 13:15:30, bobv wrote:

Check the formatting of that @link.


Done.

http://gwt-code-reviews.appspot.com/714801/diff/48001/6012#newcode67
user/test/com/google/gwt/resources/client/DataResourceDoNotEmbedTest.java:67:
// Skip this test for browsers which do not support data URLs
On 2010/08/03 13:15:30, bobv wrote:

return; here since it's an early out.



Remove the else block.  Apply in the other test class.


Done.

http://gwt-code-reviews.appspot.com/714801/diff/48001/6013
File
user/test/com/google/gwt/resources/client/DataResourceMimeTypeTest.java
(right):

http://gwt-code-reviews.appspot.com/714801/diff/48001/6013#newcode22
user/test/com/google/gwt/resources/client/DataResourceMimeTypeTest.java:22:
* Tests for code@link DataResource.MimeType @MimeType}/code resource
On 2010/08/03 13:15:30, bobv wrote:

Weird formatting with the @link here.


Done.

After fixing this numerous times, I figured out what the problem was:
the fact that I had wrapped the {...@link ...} in code tags was causing the
Eclipse auto formatter (CTRL-SHIFT-F) to remove the beginning '{'.

I removed unncessary code tags in both files and auto-format is
happier.

http://gwt-code-reviews.appspot.com/714801/diff/48001/6013#newcode27
user/test/com/google/gwt/resources/client/DataResourceMimeTypeTest.java:27:
static interface Resources extends ClientBundle {
On 2010/08/03 13:15:30, bobv wrote:

interfaces are implicitly static


Done.

http://gwt-code-reviews.appspot.com/714801/diff/48001/6013#newcode34
user/test/com/google/gwt/resources/client/DataResourceMimeTypeTest.java:34:
static final String FOUR_ZEROS_SOURCE = fourZeros.dat;
On 2010/08/03 13:15:30, bobv wrote:

Remove the static final modifiers, since that's implicit.


Done.

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-08-01 Thread fredsa 

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-07-31 Thread fredsa 

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-07-31 Thread fredsa 

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-07-29 Thread fredsa 

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-07-29 Thread bobv 

LGTM with minor changes.


http://gwt-code-reviews.appspot.com/714801/diff/10001/11001
File user/src/com/google/gwt/resources/client/DataResource.java (right):

http://gwt-code-reviews.appspot.com/714801/diff/10001/11001#newcode41
user/src/com/google/gwt/resources/client/DataResource.java:41: public
@interface DoNotEmbed {
Make both of these annotations @Documented, since they'll affect the
behavior of the resource.

http://gwt-code-reviews.appspot.com/714801/diff/10001/11002
File user/src/com/google/gwt/resources/ext/ResourceContext.java (right):

http://gwt-code-reviews.appspot.com/714801/diff/10001/11002#newcode89
user/src/com/google/gwt/resources/ext/ResourceContext.java:89: String
deploy(URL resource, String mimeType, boolean xhrCompatible)
Rename the xhrCompatible parameter to forceExternal and update the
JavaDoc to match.

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-07-29 Thread fredsa 


http://gwt-code-reviews.appspot.com/714801/diff/10001/11001
File user/src/com/google/gwt/resources/client/DataResource.java (right):

http://gwt-code-reviews.appspot.com/714801/diff/10001/11001#newcode41
user/src/com/google/gwt/resources/client/DataResource.java:41: public
@interface DoNotEmbed {
On 2010/07/29 13:19:36, bobv wrote:

Make both of these annotations @Documented, since they'll affect the

behavior of

the resource.


Done.

http://gwt-code-reviews.appspot.com/714801/diff/10001/11002
File user/src/com/google/gwt/resources/ext/ResourceContext.java (right):

http://gwt-code-reviews.appspot.com/714801/diff/10001/11002#newcode89
user/src/com/google/gwt/resources/ext/ResourceContext.java:89: String
deploy(URL resource, String mimeType, boolean xhrCompatible)
On 2010/07/29 13:19:36, bobv wrote:

Rename the xhrCompatible parameter to forceExternal and update the

JavaDoc to

match.


Done.

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-07-29 Thread fredsa 

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-07-26 Thread fredsa 

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-07-26 Thread fredsa 

http://gwt-code-reviews.appspot.com/714801/show

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

[gwt-contrib] Re: Add two new ClientBundle annotations: (issue714801)

2010-07-26 Thread fredsa 


http://gwt-code-reviews.appspot.com/714801/diff/3001/4001
File user/src/com/google/gwt/resources/client/ClientBundle.java (right):

http://gwt-code-reviews.appspot.com/714801/diff/3001/4001#newcode2
user/src/com/google/gwt/resources/client/ClientBundle.java:2: *
Copyright 2010 Google Inc.
On 2010/07/26 13:56:37, bobv wrote:

Copyright dates are not updated.


Done.

http://gwt-code-reviews.appspot.com/714801/diff/3001/4001#newcode55
user/src/com/google/gwt/resources/client/ClientBundle.java:55: public
@interface MimeType {
On 2010/07/26 13:56:37, bobv wrote:

Move these annotations into DataResource, since they're not used by

other

resource types.


Done.

Also renamed the tests from Resource*Test - DataResource*Test

http://gwt-code-reviews.appspot.com/714801/diff/3001/4002
File user/src/com/google/gwt/resources/ext/ResourceContext.java (right):

http://gwt-code-reviews.appspot.com/714801/diff/3001/4002#newcode72
user/src/com/google/gwt/resources/ext/ResourceContext.java:72: String
deploy(URL resource, String mimeType, boolean xhrCompatible)
On 2010/07/26 13:56:37, bobv wrote:

This is a public API that there are known users of.


I'll leave the original method @deprecated

http://gwt-code-reviews.appspot.com/714801/diff/3001/4003
File
user/src/com/google/gwt/resources/rebind/context/AbstractResourceContext.java
(right):

http://gwt-code-reviews.appspot.com/714801/diff/3001/4003#newcode99
user/src/com/google/gwt/resources/rebind/context/AbstractResourceContext.java:99:
? resource.openConnection().getContentType() : mimeType,
On 2010/07/26 13:56:37, bobv wrote:

Extract to local variable to improve formatting.


Done.

http://gwt-code-reviews.appspot.com/714801/show

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