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() { On 2011/01/18 20:30:22, jat wrote:
final is meaningless on static methods, right?
Yes. Removed. 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. On 2011/01/18 20:30:22, jat wrote:
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. Decided to put it as private. We discussed leaving it as protected but the subclass route opens up a can of worms factory that I think would be more harm than good. 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: On 2011/01/18 20:30:22, jat wrote:
>80 chars, throughout the change.
Done. 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 On 2011/01/18 20:30:22, jat wrote:
Should this mention the return type?
Done. http://gwt-code-reviews.appspot.com/1296801/diff/21001/22004 File user/src/com/google/gwt/user/client/IsSupported.java (left): http://gwt-code-reviews.appspot.com/1296801/diff/21001/22004#oldcode32 user/src/com/google/gwt/user/client/IsSupported.java:32: public interface IsSupported { On 2011/01/18 21:14:25, jlabanca wrote:
Is this already in GWT 2.1.1? If so, we probably don't want to pull
it,
especially if we are just replacing it with another empty interface.
I don't think we need PartialSupport, but if we do, can it just extend IsSupported?
IsSupported isn't in 2.1.1 so we're free to change it as needed. I think some sort of annotation/marker is good since this is such a large break from GWT's norm. http://gwt-code-reviews.appspot.com/1296801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
