Skip to content
This repository was archived by the owner on Oct 7, 2021. It is now read-only.

Remove topic partitions#225

Merged
Karsten1987 merged 6 commits intomasterfrom
remove_topic_partitons
Jun 7, 2018
Merged

Remove topic partitions#225
Karsten1987 merged 6 commits intomasterfrom
remove_topic_partitons

Conversation

@rohitsalem
Copy link
Copy Markdown
Member

connects to ros2/ros2#476
To use the topic/service name directly without using the partitions.

@wjwwood wjwwood added the in progress Actively being worked on (Kanban column) label Apr 11, 2018
@rohitsalem rohitsalem force-pushed the remove_topic_partitons branch from 4e3f597 to 55ad598 Compare April 18, 2018 20:57
@rohitsalem rohitsalem added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Apr 18, 2018
@Karsten1987 Karsten1987 changed the title Remove topic partitons Remove topic partitions Apr 18, 2018
@rohitsalem rohitsalem force-pushed the remove_topic_partitons branch from 55ad598 to e5b053e Compare April 25, 2018 23:31
Copy link
Copy Markdown
Member

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

lgtm

if (!process_service_name(
service_name_.c_str(), avoid_ros_namespace_conventions, service_str,
request_partition_name, response_partition_name))
request_str, response_str))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not pass request_topic_name and response_topic_name directly here?

request_topic_name = service_str + "Request";
response_topic_name = service_str + "Reply";

request_topic_name = request_str;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see comment above. Doesn't need to copy

if (!process_service_name(
service_name_.c_str(), avoid_ros_namespace_conventions, service_str,
request_partition_name, response_partition_name))
request_str, response_str))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above

@Karsten1987 Karsten1987 force-pushed the remove_topic_partitons branch from bd02b53 to 0f09573 Compare May 14, 2018 18:42
@Karsten1987 Karsten1987 merged commit d9d4db6 into master Jun 7, 2018
@Karsten1987 Karsten1987 deleted the remove_topic_partitons branch June 7, 2018 21:12
@wjwwood wjwwood removed the in review Waiting for review (Kanban column) label Jun 7, 2018
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.

4 participants