LGTM with nits.

http://gwt-code-reviews.appspot.com/1296801/diff/21001/22002
File user/src/com/google/gwt/canvas/client/Canvas.java (right):

http://gwt-code-reviews.appspot.com/1296801/diff/21001/22002#newcode61
user/src/com/google/gwt/canvas/client/Canvas.java:61: public static
final boolean isSupported() {
final is meaningless on static methods, right?

http://gwt-code-reviews.appspot.com/1296801/diff/21001/22002#newcode74
user/src/com/google/gwt/canvas/client/Canvas.java:74: * Protected
constructor. Use {@link #createIfSupported()} to create a Canvas.
If this is protected, you should also talk about how subclasses should
use it.  For example, you might want to say that they should never have
a public constructor, or else you need to specify that this can throw
exceptions if called when not supported.

If you don't expect subclasses, it might be better to make this private
for now, and then you can define what subclasses do when it becomes
useful to subclass.

http://gwt-code-reviews.appspot.com/1296801/diff/21001/22003
File user/src/com/google/gwt/dom/client/PartialSupport.java (right):

http://gwt-code-reviews.appspot.com/1296801/diff/21001/22003#newcode22
user/src/com/google/gwt/dom/client/PartialSupport.java:22: * By
convention, classes annotated with PartialSupport will provide two
static methods:
80 chars, throughout the change.

http://gwt-code-reviews.appspot.com/1296801/diff/21001/22003#newcode26
user/src/com/google/gwt/dom/client/PartialSupport.java:26: *   <li>
<code>static createIfSupported()</code> factory method that returns a
new feature
Should this mention the return type?

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

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

Reply via email to