On Mon, Mar 12, 2012 at 11:37 AM, Brian Gerkey <gerkey@willowgarage.com> wrote:
> hi Jack,
>
> On Sun, Mar 11, 2012 at 5:00 PM, Jack O'Quin <jack.oquin@gmail.com> wrote:
>> I want to update the comments in the sensor_msgs/NavSatFix message.
>> This update is a clarification which arose in the NMEA GPS driver
>> review:
>>
>>  http://ros.org/wiki/nmea_gps_driver/Reviews/2012-03-05_API_Review
>>
>> It should not break anything, but I hate to commit a change to
>> sensor_msgs without a regression test.
>>
>> Unfortunately, I can't figure out how to build common_msgs. I checked
>> out a copy and made my change. Here's the way I tried to build it
>> (without success):
>>
>>  $ roscd sensor_msgs
>>  $ mkdir build
>>  $ cd build
>>  $ cmake ..
>> -- The C compiler identification is GNU
>> -- The CXX compiler identification is GNU
>> -- Check for working C compiler: /usr/bin/gcc
>> -- Check for working C compiler: /usr/bin/gcc -- works
>> -- Detecting C compiler ABI info
>> -- Detecting C compiler ABI info - done
>> -- Check for working CXX compiler: /usr/bin/c++
>> -- Check for working CXX compiler: /usr/bin/c++ -- works
>> -- Detecting CXX compiler ABI info
>> -- Detecting CXX compiler ABI info - done
>> CMake Error at CMakeLists.txt:2 (catkin_project):
>>  Unknown CMake command "catkin_project".
>>
>>
>> CMake Warning (dev) in CMakeLists.txt:
>>  No cmake_minimum_required command is present.  A line of code such as
>>
>>    cmake_minimum_required(VERSION 2.8)
>>
>>  should be added at the top of the file.  The version specified may be lower
>>  if you wish to support older CMake versions for this project.  For more
>>  information run "cmake --help-policy CMP0000".
>> This warning is for project developers.  Use -Wno-dev to suppress it.
>>
>> -- Configuring incomplete, errors occurred!
>
> You need to configure at the stack level.  E.g.:
>
> $ cd common_msgs
> $ mkdir build
> $ cd build
> $ cmake ..
>
> After that, you can build any of the targets offered by cmake (`make
> help` to list them).  Each project is a target, which means that you
> can build everything in the sensor_msgs package:
>
> $ make sensor_msgs
>
> We hadn't considered the use case of configuring at the
> package/project level as you attempted to do.  It's doable, but would
> put extra burden on developers in terms of the boilerplate that would
> need to be in each CMakeLists.txt.  And we would need an extra set of
> regression builds to ensure that package-level configuring keeps
> working.  So I'm not inclined to support it.

Agreed.

I was making a mistake, anyway, overlaying all of common_msgs but only
building sensor_msgs. That is not really a valid test.

>> Should I commit this change, anyway? It should be safe.
>
> The change looks ok to me.  IIRC, comment changes don't change the
> md5sum, so there shouldn't be any user-visible consequences.

That's right, no MD5sum change. It just makes the intended meanings of
the header and altitude fields explicit.

Thanks for the help, Brian. I needed to get this done before Thursday,
and I'm going to be away the next few days. Will you do a
common_msgs-1.8.1 release before final freeze? I updated the
ChangeList.
-- 
 joq