[foxy-backport] fast forward of ros2 (default) to foxy #570
Conversation
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
This removes one more warning from rviz_common builds. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
…y cross platform (#565) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: ymd-stella <world.applepie@gmail.com>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
* Don't install test header files in rviz_rendering. This change started as a relatively simple try at not installing test includes when installing rviz_rendering. As you can see, it ballooned into quite a large change, so here is an explanation of why. We shouldn't install test include header files when installing the package; that just ends up on the end user system, and is a non-supported API. Worse, we don't want to compile test code in our main library. Yet rviz_rendering is currently doing both of these things. Unfortunately, it is somewhat tricky to make that code and header file private. The problem is that that test code is used in rviz_rendering, rviz_common, rviz_default_plugins, and rviz_rendering_tests. One solution might be to extract that test code into its own package, and then have all of the other packages depend on it. The problem is that that test package would both be depended on by rviz_rendering (for tests), and depends itself on rviz_rendering (for its functionality), creating a dependency loop. The solution I opted for here is to copy the test code into the appropriate packages. This leads to some duplication of functionality, but it effectively breaks the dependency loop and succeeds in eliminating the test code from our installed library. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
There was a problem hiding this comment.
I'll give thumbs up for my commits at least. But just for my understanding:
#567 should be closed in favor of this PR then, right? Also, I thought it was on the project board. But maybe I missed something here.
|
Sorry, I didn't see that. Yeah I will close #567 in favor of this one if all of the commits are ok. I think it's helpful for the original issue/pull request to be in the "needs backport" column until it is released into foxy. But that's just my take. |
I'm on the fence about it. It should be relatively harmless, but as you say, it is the riskiest of the group. If you think that the benefit of being able to cherry-pick commits between |
|
Ok, @jacobperron do you have an opinion? |
There was a problem hiding this comment.
A fast-forward is okay by me.
The potential for a regression due to #564 seems relatively low considering it is touching test files.
|
Ok, I'll do the fast-forward then. |
|
Because there's a header that was moved to public from private and therefore adds API: https://github.com/ros2/rviz/pull/564/files#diff-608c586d26f0d375668eb4e5e1e4acfc I think I'll do a minor version bump. |
I went over the pull requests merged since the foxy release, and I believe all of them are ok from an ABI perspective. There were some build system changes, but I believe they should be backwards compatible, but I'm not 100% sure of that.
I'll for feedback on that point for some of the individual commits, but here is a list of all of them:
@jacobperron FYI