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.

Sign up for GitHub

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

Jump to bottom

De-deplicate type_support_map.h header #81

Merged
merged 11 commits into from
Sep 11, 2020
Merged

De-deplicate type_support_map.h header #81

merged 11 commits into from
Sep 11, 2020

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented Aug 3, 2020

Fixes: #79

@j-rivero j-rivero force-pushed the jrivero/deduplicate branch 2 times, most recently from c2b69e8 to fab24a5 Compare August 4, 2020 16:27
@j-rivero j-rivero requested a review from dirk-thomas August 4, 2020 16:44
@dirk-thomas
Copy link
Member

The removal of the public header file rosidl_typesupport_cpp/type_support_map.h is breaking public API. I would suggest to avoid that kind of breakage and keep the header in place (redirecting to the C header) and add a deprecation warning in case it is still used.

@j-rivero j-rivero self-assigned this Aug 4, 2020
@j-rivero j-rivero force-pushed the jrivero/deduplicate branch 2 times, most recently from b0d43b4 to d678bed Compare August 13, 2020 18:32
@j-rivero
Copy link
Contributor Author

@ros-pull-request-builder run tests please

@j-rivero j-rivero mentioned this pull request Aug 13, 2020
Closed
@dirk-thomas
Copy link
Member

@ros-pull-request-builder retest this please (using custom job config from ros-infrastructure/ros_buildfarm#828)

@j-rivero j-rivero force-pushed the jrivero/deduplicate branch from 1f77561 to 9be02fe Compare August 14, 2020 18:04
@j-rivero j-rivero requested a review from dirk-thomas August 14, 2020 18:09
@j-rivero
De-deplicate type_support_map.h header
768ccbe 
Fixes: #79
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Use rosidl_typesupport_c in CMake
af99fbf 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Revert the removal of type_support_map.h file to avoid breaking API
a74937a 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Add compiler warning
9e81c05 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Fix uncrustify report
cfc226b 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Declare rosidl_typesupport_c as a build_depend
1037a32 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Revert changes in empy files
a8696a9 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Reorder headers to keep alphabetic order
5f0118a 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Revert missing changes in empy files
6e22ee2 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Need to export rosidl_typesupport_c as build depend
5f58fa2 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Right alpha order for headers
4d9a6fa 
Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Copy link
Contributor Author

ough the signoff amend command re-push a good bunch of commits, the ones did since your last comments are 4d9a6fa and
5f58fa2

@j-rivero j-rivero merged commit 29da73a into master Sep 11, 2020
@delete-merged-branch delete-merged-branch bot deleted the jrivero/deduplicate branch September 11, 2020 17:08
@dirk-thomas
Copy link
Member

Did you happen to run any CI builds for this change?

@j-rivero
Copy link
Contributor Author

j-rivero commented Sep 11, 2020 •
edited
Loading

Did you happen to run any CI builds for this change?

eh uh ... totally forgot to run ros2 ci jobs, I just relied on the github CI mark. Go to check now.

@j-rivero
Copy link
Contributor Author

eh uh ... totally forgot to run ros2 ci jobs, I just relied on the github CI mark. Go to check now.

Testing up to rclcpp, running rclcpp tests:

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

@j-rivero j-rivero mentioned this pull request Sep 14, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dirk-thomas dirk-thomas dirk-thomas approved these changes

Assignees

@j-rivero j-rivero

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Same header file hosted in two packages: type_support_map.h
2 participants
@j-rivero @dirk-thomas