Skip to content

Deprecating all C headers#135

Merged
ahcorde merged 2 commits intoros2:rollingfrom
CursedRock17:deprecate_c_headers
Jul 18, 2024
Merged

Deprecating all C headers#135
ahcorde merged 2 commits intoros2:rollingfrom
CursedRock17:deprecate_c_headers

Conversation

@CursedRock17
Copy link
Copy Markdown
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 <mtglucas1@gmail.com>
@CursedRock17 CursedRock17 requested a review from gbiggs as a code owner July 12, 2024 15:27
@@ -1,342 +1,22 @@
// Copyright 2008, Willow Garage, Inc. All rights reserved.
// Copyright 2024 Open Source Robotics Foundation, Inc.
Copy link
Copy Markdown
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
Copy Markdown
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 <mtglucas1@gmail.com>
@ahcorde
Copy link
Copy Markdown
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
Copy Markdown
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
@CursedRock17
Copy link
Copy Markdown
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
Copy Markdown
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