-
Notifications
You must be signed in to change notification settings - Fork 90
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
Take and return new RMW_DURATION_INFINITE correctly #288
Take and return new RMW_DURATION_INFINITE correctly #288
Conversation
459592b
to
82b33fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM @emersonknapp!
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
c1ed9b6
to
5c6b30a
Compare
@eboasson @ivanpauno friendly ping - the dependent RMW PR has been merged, so this is ready to go. I notice that the actions here are not working since November (https://github.com/ros2/rmw_cyclonedds/actions?query=event:push+branch:master) , and I'm not sure I want to mess around with your specific CI setup - so what is the next step? |
@emersonknapp can you run CI for this? for the Rpr checker to pass you would need someone releasing rmw to rolling. |
I think we should aim for at least the Rpr job to go green (along with the usual https://ci.ros2.org jobs). I can do an rmw release a little later. |
Gist: https://gist.githubusercontent.com/emersonknapp/6c22d23c3fac5b8ced3605986aa33a46/raw/fb33f2320709a25cfb79267321a16e57dc0f4eb9/ros2.repos |
FYI, I did do an |
@ros-pull-request-builder retest this please |
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
Depends on ros2/rmw#301
Alternative to #287
Meets the new API expectations introduced in ros2/rmw#301 - to accept and return
RMW_DURATION_INFINITE
for cross-implementation consistency.