Initial eigen3_cmake_module package #1
Conversation
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
|
@sloretz who should we ping to review this for merge & buildfarm release? |
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
| Adds a custom find module for Eigen3 | ||
|
|
||
| This package adds a [CMake find module](https://cmake.org/cmake/help/v3.14/manual/cmake-developer.7.html#find-modulesjj) for [Eigen3](https://eigen.tuxfamily.org/dox/) that sets [standard CMake variables](https://cmake.org/cmake/help/v3.5/manual/cmake-developer.7.html#standard-variable-names). | ||
| This enables `ament_export_dependencies(Eigen3)` and `ament_target_dependencies(my_target Eigen3)`. |
There was a problem hiding this comment.
Is it possible to give a little more context here on this issue? In your explanation you mentioned the reasoning behind this package is that ament process for adding eigen3 as a dependency may not work in Ubuntu bionic. Would the user be able to recognize that that's the problem when their "library or executable using Eigen3" failed to compile? Would someone searching for a fix be able to recognize that this is for that exact issue?
For your audience to be convinced that this is the solution to their problem, I just want to make sure that the "solution" mentioned here is enough to do so.
Maybe "This enables ... in the event that ... (it fails because x)"
|
|
||
| ## Using this package | ||
|
|
||
| This section assumes you're using [ament_cmake](https://github.com/ament/ament_cmake). |
There was a problem hiding this comment.
From what I can gather, it seems like the whole purpose behind this package and the problem/solution here is 100% due to/reliant on ament. Would someone who wasn't using ament come across this problem? If yes, is there any other way to solve the problem without using ament?
"... assumes you're using.." makes it sound like there are other options, which might be misleading or confusing (or correct if my understanding of the situation is wrong!) Hinting at other options but not elaborating on them might be confusing. If there are other ways you might want to say something like:
"You can follow the ament syntax below to accomplish the same thing using < another build tool of choice >"
Otherwise I would either remove the statement or change it to convey "This section requires ament"
There was a problem hiding this comment.
Someone not using ament_cmake would also notice that Eigen3 uses non-standard variable names. It's less of a problem for them since they have to do everything manually already. ament_cmake packages have an issue because they use convenience functions (ament_target_dependencies(), ament_export_dependencies()) that make assumptions about the variable names.
I added instructions for non-ament_cmake users in
3c1f634 in #2
This adds a package that provides a find module for
Eigen3that sets standard cmake variables.