Prepare dependencies for bouncy release. #41
Conversation
nuclearsandwich
commented
Jun 21, 2018
nuclearsandwich
commented
- Add build dependency on each current rmw implementations
- Add a comment indicating how runtime dependencies will be handled for debian packaging.
* Add build dependency on each current rmw implementations * Add a comment indicating how runtime dependencies will be handled for debian packaging.
rmw_implementation/package.xml
Outdated
| The debian/control template for this package will be modified to depend on either | ||
| rmw_fastrtps_cpp or rmw_connext_cpp or rmw_opensplice_cpp with rmw_fastrtps_cpp being | ||
| the default choice if no other rmw implementation is currently installed. | ||
| <depend>rmw_fastrtps_cpp</depend> |
There was a problem hiding this comment.
Should I comment this out since there won't be an absolute dependency on rmw_fastrtps_cpp or leave it in for from-source builds?
There was a problem hiding this comment.
Since the Debian package shouldn't have a hard dependency on FastRTPS it shouldn't be commented in.
I am not sure if the comment here is a good idea. It solely describes packaging customizations. Having them duplicated here just adds the risk of them getting out-of-sync while not adding much value here.
There was a problem hiding this comment.
Since the Debian package shouldn't have a hard dependency on FastRTPS it shouldn't be commented in.
I don't quite follow the outcome of this sentence. Should I restore the dependency which will be modified rather than removed at packaging time or leave it commented while modifying the comment above it to reduce bitrot?
There was a problem hiding this comment.
I would remove this whole comment since it is packaging specific.
rmw_implementation/package.xml
Outdated
| @@ -11,11 +11,20 @@ | |||
|
|
|||
| <build_depend>rcutils</build_depend> | |||
| <build_depend>rmw</build_depend> | |||
| <build_depend>rmw_connext_cpp</build_depend> | |||
| <build_depend>rmw_fastrtps_cpp</build_depend> | |||
| <build_depend>rmw_opensplice_cpp</build_depend> | |||
There was a problem hiding this comment.
Why should these be added upstream? The group dependency below already covers them.
There was a problem hiding this comment.
They're redundant with upstream's use of the group tag so they don't change upstream behavior. The other dependency work for this repository will happen after package.xmls are handled so with these changes upstream no package.xml overlay is needed in the release repo.
But since they are redundant I can instead add them to an overlay or patch in the release repositor if that is preferred.
There was a problem hiding this comment.
It makes sense to add them here is that avoid patching the manifest in the release repo since it doesn't affect the behavior.
Please add a comment above these three line describing why they are in there.
Updated verbiage pulled from ros2/rmw_implementation#41
