Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Add function for getting clients by node #361

Merged
merged 2 commits into from
Jul 9, 2019

Conversation

jacobperron
Copy link
Member

Connects to ros2/rmw#179

node_name,
node_namespace,
service_names_and_types,
"Reply");
Copy link
Member

Choose a reason for hiding this comment

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

I know this is the last review of the set, but I just considered that maybe the "Reply" and "Request" should be some sort of static const, so that we don't have to update them if something changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's a good idea. What about #define them in names_and_types_helpers.hpp?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you're okay with it, I can do this refactoring in a follow-up set of PRs, since this can be done in all of the rmw implementations.

Copy link
Member

Choose a reason for hiding this comment

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

I'm good with the followup.

Signed-off-by: Jacob Perron <[email protected]>
@jacobperron jacobperron force-pushed the jacob/get_clients_by_node branch from 585b287 to f7fd64f Compare July 2, 2019 17:30
Copy link
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

LGTM

@jacobperron jacobperron merged commit 70881fc into master Jul 9, 2019
@delete-merged-branch delete-merged-branch bot deleted the jacob/get_clients_by_node branch July 9, 2019 16:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants