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

typesupport_tests needs to be updated to C++17 #137

Merged
merged 4 commits into from
Mar 28, 2023

Conversation

CursedRock17
Copy link
Contributor

Similar to PR #131 most of ros 2 support should be pushed towards C++17 and rosidl_typesupport_tests should be included to be consistent. Also, this was causing errors on MacOS and if it's such a miniscule change that doesn't cause issues in other places, there is no reason not to add C++17 support.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Can you please rebase this onto the latest? It has some unrelated changes in it, and it is making it difficult to review. Thanks.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Besides what I've pointed out inline, this block should also be before the one that does add_compile_options. It probably doesn't matter too much in practice, but all of our other packages have it that way.

rosidl_typesupport_tests/CMakeLists.txt Show resolved Hide resolved
@clalancette
Copy link
Contributor

clalancette commented Mar 27, 2023

CI:

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

Signed-off-by: CursedRock17 <mtglucas1@gmail.com>
This reverts commit 0ea986b.

Signed-off-by: CursedRock17 <mtglucas1@gmail.com>
Signed-off-by: CursedRock17 <mtglucas1@gmail.com>
@clalancette clalancette merged commit 9f1e466 into ros2:rolling Mar 28, 2023
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

2 participants