> ========================================================================
> http://mondrian.corp.google.com/file/8544235///depot/googleclient/gears/opensource/gears/geolocation/geolocation_test.cc?a=2
> File 
> //depot/googleclient/gears/opensource/gears/geolocation/geolocation_test.cc 
> (snapshot 2)
> ------------------------------------
> Line 627: }
> I think it's nice to have a blank line between each function definition.
Done

> ========================================================================
> http://mondrian.corp.google.com/file/8544235///depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js?a=2
> File 
> //depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js
>  (snapshot 2)
> ------------------------------------
> Line 447: function testMockGpsDevice() {
> In a different CL, could we please add a couple of tests for the new reverse
> geocoding stuff? We could configure the mock GPS device with some coords that
> are known to reverse geocode nicely and then set gearsRequestAddress to true..
Yup, the TODO is already there.

> ========================================================================
> http://mondrian.corp.google.com/file/8544235///depot/googleclient/gears/opensource/gears/test/tester/assert.js?a=1
> File //depot/googleclient/gears/opensource/gears/test/tester/assert.js 
> (snapshot 1)
> ------------------------------------
> Line 41: var isAndroid = 
> google.gears.factory.getBuildInfo().indexOf('android') > -1;
> Please remove this line, as it's already there (line 45).
This was a merge problem - fixed.

New snapshot uploaded.

Reply via email to