Skip to content

Remove MANUAL_BY_NODE liveliness API#178

Merged
ivanpauno merged 5 commits intomasterfrom
ivanpauno/remove-manual-by-node-liveliness
May 12, 2020
Merged

Remove MANUAL_BY_NODE liveliness API#178
ivanpauno merged 5 commits intomasterfrom
ivanpauno/remove-manual-by-node-liveliness

Conversation

@ivanpauno
Copy link
Copy Markdown
Member

@ivanpauno ivanpauno commented May 7, 2020

ivanpauno added 2 commits May 7, 2020 11:05
@ivanpauno
Remove MANUAL_BY_NODE liveliness API
fee9dca 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Undo unrelated change
23b3183 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno self-assigned this May 7, 2020
@rotu
Copy link
Copy Markdown
Collaborator

rotu commented May 7, 2020 •
edited
Loading

This is not yet deprecated. Shouldn't we be deprecating instead?
ros2/rmw#227 (comment)

wjwwood
wjwwood reviewed May 7, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

lgtm, though I'm not sure if the master branch needs to support more than one version simultaneously, it might require ifdef logic instead.

eboasson
eboasson reviewed May 8, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@eboasson eboasson left a comment

Choose a reason for hiding this comment

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

One little detail ...

Otherwise LGTM given the decision to remove it. @wjwwood We're no longer maintaining compatibility with Dashing and Eloquent and now follow the standard model of release branches, so no need for conditional stuff.

Comment thread rmw_cyclonedds_cpp/src/rmw_node.cpp
@rotu rotu mentioned this pull request May 8, 2020
Merged
@ivanpauno
Set liveliness to RMW_QOS_POLICY_LIVELINESS_UNKNOWN when using DDS_LI…
1b5f10c 
…VELINESS_MANUAL_BY_PARTICIPANT

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@rotu rotu mentioned this pull request May 8, 2020
Merged
@ivanpauno
Fix error
801e2fc 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Copy link
Copy Markdown
Member Author

ivanpauno commented May 8, 2020

lgtm, though I'm not sure if the master branch needs to support more than one version simultaneously, it might require ifdef logic instead.

Master is used only for foxy and later.
dashing-eloquent is the one used for previous distrubutions.

wjwwood
wjwwood reviewed May 8, 2020
View reviewed changes
Comment thread rmw_cyclonedds_cpp/src/rmw_node.cpp
@ivanpauno
Add default case to switch statement
7f8a451 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno merged commit 8fafb3e into master May 12, 2020
@delete-merged-branch delete-merged-branch Bot deleted the ivanpauno/remove-manual-by-node-liveliness branch May 12, 2020 17:51
@rotu
Copy link
Copy Markdown
Collaborator

rotu commented May 12, 2020

FYI @eboasson, @jacobperron, this is not backwards compatible with 0.6.0 so we need a new release.

@jacobperron
Copy link
Copy Markdown
Member

jacobperron commented May 12, 2020

A new release of rmw_cyclonedds is forthcoming (with releases for all other packages involved in ros2/rmw#227).

@ivanpauno
Copy link
Copy Markdown
Member Author

ivanpauno commented May 13, 2020

Release done: ros/rosdistro#24849.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eboasson eboasson eboasson left review comments

@wjwwood wjwwood wjwwood approved these changes

Assignees

@ivanpauno ivanpauno

Labels

in review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ivanpauno @rotu @jacobperron @wjwwood @eboasson