[Merge] lp:~thomas-voss/location-service/fix-1478750 into lp:location-service

Alberto Mardegan alberto.mardegan at canonical.com
Sat Oct 17 11:28:26 UTC 2015


Review: Approve

Just a small inline comment about the tests, but feel free to keep the code as it is now if you  prefer so.

Diff comments:

> 
> === removed symlink 'debian/libubuntu-location-service2.install.amd64'
> === target was u'libubuntu-location-service2.install.with-gps'
> === removed symlink 'debian/libubuntu-location-service2.install.armhf'
> === target was u'libubuntu-location-service2.install.with-gps'
> === removed symlink 'debian/libubuntu-location-service2.install.i386'
> === target was u'libubuntu-location-service2.install.with-gps'
> === renamed file 'debian/libubuntu-location-service2.install' => 'debian/libubuntu-location-service3.install'
> === added symlink 'debian/libubuntu-location-service3.install.amd64'
> === target is u'libubuntu-location-service3.install.with-gps'
> === added symlink 'debian/libubuntu-location-service3.install.armhf'
> === target is u'libubuntu-location-service3.install.with-gps'
> === added symlink 'debian/libubuntu-location-service3.install.i386'
> === target is u'libubuntu-location-service3.install.with-gps'
> === renamed file 'debian/libubuntu-location-service2.install.with-gps' => 'debian/libubuntu-location-service3.install.with-gps'
> === modified file 'tests/position_test.cpp'
> --- tests/position_test.cpp	2014-06-20 07:40:34 +0000
> +++ tests/position_test.cpp	2015-07-30 08:17:25 +0000
> @@ -24,14 +24,14 @@
>  TEST(Position, AllFieldsAreInvalidForDefaultConstructor)
>  {
>      cul::Position p;
> -    EXPECT_FALSE(p.altitude);
> -    EXPECT_FALSE(p.accuracy.vertical);
> +    EXPECT_FALSE(static_cast<bool>(p.altitude));

I would prefer to be more explicit and say:
  EXPECT_EQ(0, p.altitude);
  EXPECT_EQ(0, p.accuracy.vertical);

> +    EXPECT_FALSE(static_cast<bool>(p.accuracy.vertical));
>  }
>  
>  TEST(Position, InitWithLatLonGivesValidFieldsForLatLon)
>  {
>      cul::Position p{cul::wgs84::Latitude{}, cul::wgs84::Longitude{}};
> -    EXPECT_FALSE(p.altitude);
> +    EXPECT_FALSE(static_cast<bool>(p.altitude));
>  }
>  
>  TEST(Position, InitWithLatLonAltGivesValidFieldsForLatLonAlt)


-- 
https://code.launchpad.net/~thomas-voss/location-service/fix-1478750/+merge/266360
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service.



More information about the Ubuntu-reviews mailing list