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

Deprecating all C headers #135

Merged
merged 2 commits into from
Jul 18, 2024
Merged

Conversation

CursedRock17
Copy link
Contributor

This pull request is another and possibly the last needed addition to complete the breakdown of PR #120 in which all C headers are deprecated to C++ headers to solve this issue:

But there is another problem here, and that is that message_filters isn't a "proper" C++ package. In particular, the header files still have the .h extension, and so our linters consider them to be C headers, not C++ headers

The PR appears to be super messy because I guess renaming files counts as making a whole new file in Git's eyes, but really each old file was renamed to have the .hpp tag with the correct header guard, then a new file .h was created with a warning message to use the C++ style header, so not too bad.

Signed-off-by: CursedRock17 <[email protected]>
@@ -1,342 +1,22 @@
// Copyright 2008, Willow Garage, Inc. All rights reserved.
// Copyright 2024 Open Source Robotics Foundation, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Copyright 2024 Open Source Robotics Foundation, Inc.
// Copyright 2024 Open Source Robotics Foundation, Inc.
// All rights reserved.

Copy link
Contributor

Choose a reason for hiding this comment

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

to be consistent with the rest of headers, here and in the rest of files

Signed-off-by: CursedRock17 <[email protected]>
@ahcorde
Copy link
Contributor

ahcorde commented Jul 16, 2024

I added rviz, geometry2, image_common and point_cloud_transport changes too

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

@ahcorde
Copy link
Contributor

ahcorde commented Jul 17, 2024

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

@ahcorde ahcorde merged commit 5551518 into ros2:rolling Jul 18, 2024
2 checks passed
@CursedRock17
Copy link
Contributor Author

Since this pull request was meant to be the last of pull request #120, I feel we should be able to close both #120 and its respective issue of #29 since linters have been totally added. We just have to revisit this pull request in I believe L-turtle to remove the deprecations.

@ahcorde
Copy link
Contributor

ahcorde commented Jul 19, 2024

Since this pull request was meant to be the last of pull request #120, I feel we should be able to close both #120 and its respective issue of #29 since linters have been totally added. We just have to revisit this pull request in I believe L-turtle to remove the deprecations.

See this comment #120

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.

2 participants