http://gwt-code-reviews.appspot.com/1451811/diff/2026/user/src/com/google/gwt/geolocation/client/Geolocation.java File user/src/com/google/gwt/geolocation/client/Geolocation.java (right):
http://gwt-code-reviews.appspot.com/1451811/diff/2026/user/src/com/google/gwt/geolocation/client/Geolocation.java#newcode160 user/src/com/google/gwt/geolocation/client/Geolocation.java:160: if (isSupported()) { On 2011/06/08 21:58:44, jlabanca wrote:
This is inverted. getIfSupported() returns null if isSupported() is
true. Whoops! Done. http://gwt-code-reviews.appspot.com/1451811/diff/3004/user/src/com/google/gwt/geolocation/client/Geolocation.java#newcode191 user/src/com/google/gwt/geolocation/client/Geolocation.java:191: opt.enableHighAccuracy = optio...@com.google.gwt.geolocation.client.Geolocation.PositionOptions::enableHighAccuracy; On 2011/06/08 21:58:44, jlabanca wrote:
inverted conditional? You want to execute this block if options is
not null. Done. http://gwt-code-reviews.appspot.com/1451811/diff/3004/user/src/com/google/gwt/geolocation/client/Geolocation.java#newcode212 user/src/com/google/gwt/geolocation/client/Geolocation.java:212: * </p> On 2011/06/08 21:58:44, jlabanca wrote:
This comment isn't needed. You can't call this method is geolocation
isn't
supported. Same for the other methods.
Done. http://gwt-code-reviews.appspot.com/1451811/diff/3004/user/test/com/google/gwt/geolocation/client/GeolocationTest.java File user/test/com/google/gwt/geolocation/client/GeolocationTest.java (right): http://gwt-code-reviews.appspot.com/1451811/diff/3004/user/test/com/google/gwt/geolocation/client/GeolocationTest.java#newcode45 user/test/com/google/gwt/geolocation/client/GeolocationTest.java:45: assertNull(geolocation); On 2011/06/08 21:58:44, jlabanca wrote:
This is inverted. It won't be null if supported. But at least it
matches the
class its testing :p
Done. http://gwt-code-reviews.appspot.com/1451811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
