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