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

Add TRACETOOLS_ prefix to tracepoint-related public macros #56

Conversation

christophebedard
Copy link
Member

@christophebedard christophebedard commented Apr 9, 2023

Closes #18

This adds a TRACETOOLS_ prefix to tracepoint-related public macros: TRACEPOINT(), TRACEPOINT_ENABLED(), and DO_TRACEPOINT(). Since this is part of the public API, I left the current macros there and marked them as deprecated by defining and calling an empty deprecated function. Downstream PRs:

It also adds a leading underscore to internal macros, especially in public headers.

Signed-off-by: Christophe Bedard bedard.christophe@gmail.com

action-ros-ci-repos-override: https://gist.githubusercontent.com/christophebedard/0f809c29f26685eaee9365a2f5bfaa17/raw/43889baefe7f2aa9803a5204a9c23c8c88d9e899/ros2.repos

@christophebedard
Copy link
Member Author

RMW is feature-frozen now, but if we merge this, then is it considered a bug fix to merge the corresponding rmw_* PRs? 🤔

@christophebedard
Copy link
Member Author

Holding off on this until after Iron branches off from Rolling (April 24th).

@christophebedard christophebedard marked this pull request as draft April 11, 2023 17:27
@christophebedard christophebedard force-pushed the christophebedard/add-namespace-prefix-tracetools-macros branch from d614a0e to 5281e3f Compare May 3, 2023 20:08
@christophebedard christophebedard marked this pull request as ready for review May 3, 2023 20:09
@christophebedard christophebedard force-pushed the christophebedard/add-namespace-prefix-tracetools-macros branch from 5281e3f to d03719a Compare May 3, 2023 20:19
@christophebedard
Copy link
Member Author

This is ready for review.

@christophebedard
Copy link
Member Author

christophebedard commented May 3, 2023

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

@christophebedard christophebedard force-pushed the christophebedard/add-namespace-prefix-tracetools-macros branch from d03719a to c061403 Compare May 3, 2023 20:32
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
@christophebedard christophebedard force-pushed the christophebedard/add-namespace-prefix-tracetools-macros branch from c061403 to ab83aca Compare June 2, 2023 17:15
@christophebedard
Copy link
Member Author

@mjcarroll friendly ping for a review now that the Iron dust has settled a bit

@mjcarroll
Copy link
Member

@christophebedard looks good, but we can probably use another round of CI as it has been over a month now.

@christophebedard
Copy link
Member Author

New CI:

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

@christophebedard
Copy link
Member Author

christophebedard commented Jun 8, 2023

I see the same rclcpp.test_executors failure in a bunch of other tests jobs, so this looks good to me.

I can only merge this PR, not the other ones. @mjcarroll if I merge this one, can you merge the other ones at the same time?

@mjcarroll
Copy link
Member

I see the same rclcpp.test_executors failure in a bunch of other tests, so this looks good to me.

Yep, I'm looking into that as well.

can you merge the other ones at the same time?

Yep, just shoot me a message when you are ready.

@christophebedard christophebedard merged commit cdde911 into rolling Jun 8, 2023
9 checks passed
@christophebedard christophebedard deleted the christophebedard/add-namespace-prefix-tracetools-macros branch June 8, 2023 17:38
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.

Add namespace/prefix to tracetools macros
2 participants