Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

redo use _TARGETS over deprecated _INTERFACES over classic CMake variables #251

Merged

Conversation

dirk-thomas
Copy link
Contributor

Redo of #249. Related to ros2/ros2#904.

@dirk-thomas dirk-thomas self-assigned this Apr 24, 2020
@dirk-thomas dirk-thomas force-pushed the dirk-thomas/use-targets-over-interfaces-over-classic-vars-2 branch from bc2d832 to f459e00 Compare April 25, 2020 06:06
@dirk-thomas dirk-thomas marked this pull request as ready for review April 27, 2020 00:54
message(DEPRECATION
"Package ${package_name} is exporting the variable "
"${package_name}_INTERFACES which is deprecated, it should export
${package_name}_TARGETS instead")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dirk-thomas we need an issue to follow-up with this deprecation once Foxy is out.

@dirk-thomas
Copy link
Contributor Author

Full CI builds:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Full CI builds (without this PR but all other PRs related to modern CMake):

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

So I will go ahead and merge the other PRs but keep looking into why this one has a problem on Windows.

@dirk-thomas
Copy link
Contributor Author

The latest commit fixes the Windows build: Build Status

…ables

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas force-pushed the dirk-thomas/use-targets-over-interfaces-over-classic-vars-2 branch from 7a2d7fb to bd530eb Compare April 29, 2020 22:32
@dirk-thomas
Copy link
Contributor Author

CI builds without tests:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Windows build testing rviz_default_plugins: Build Status

Copy link
Contributor

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to review this, but I think I lack some of the context needed to know if this is right. I don't see anything obviously wrong.

@dirk-thomas
Copy link
Contributor Author

Hopefully this second time the merge is more successful 🤞

@dirk-thomas dirk-thomas merged commit 0fc4000 into master Apr 30, 2020
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/use-targets-over-interfaces-over-classic-vars-2 branch April 30, 2020 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants