Skip to content

Remove unused IntraProcessMessage#89

Merged
ivanpauno merged 2 commits intoros2:masterfrom
brawner:remove_intraprocessmessage
Mar 20, 2020
Merged

Remove unused IntraProcessMessage#89
ivanpauno merged 2 commits intoros2:masterfrom
brawner:remove_intraprocessmessage

Conversation

@brawner
Copy link
Copy Markdown
Contributor

@brawner brawner commented Mar 6, 2020

IntraProcessMessage is no longer used except as a dummy message in rclcpp tests. This removes the message from rcl_interfaces and creates an empty message in test_msgs for those rclcpp tests. rclcpp doesn't depend on common_interfaces, but it already depended on test_msgs so this doesn't change its dependency count.

If reusing common_interfaces/empty.msg is better than creating an empty message I can switch to that instead.

@brawner brawner mentioned this pull request Mar 6, 2020
Merged
@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Mar 6, 2020

See also ros2/rclcpp#1017

@brawner
Remove unused IntraProcessMessage
4891d55 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner force-pushed the remove_intraprocessmessage branch from 68fee97 to 4891d55 Compare March 6, 2020 02:13
@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Mar 6, 2020

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

@brawner brawner mentioned this pull request Mar 6, 2020
Closed
ivanpauno
ivanpauno requested changes Mar 6, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

Can you move the Dummy msg here https://github.com/ros2/test_interface_files/tree/master/msg?
Thanks!

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Mar 6, 2020

Creating a Dummy.msg for this case seems overkill. If the tests can use an existing message definition instead that would be preferred. If they can't it would be good to state the rationale why.

@brawner
Remove Dummy in favor of test_interface_files
f9b5432 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner force-pushed the remove_intraprocessmessage branch from 53da0cb to f9b5432 Compare March 6, 2020 20:02
@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Mar 6, 2020

Thanks @ivanpauno for the suggestion. It looks like there is already an Empty message there, so I'm going to use that instead for rclcpp. Latest commit removes Dummy.

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

@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Mar 19, 2020

Retesting:

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

@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Mar 20, 2020

@ivanpauno @wjwwood The tests now pass. If someone could merge this for me, I would appreciate it.

@brawner brawner mentioned this pull request Mar 20, 2020
Merged
@ivanpauno ivanpauno merged commit c8a34e3 into ros2:master Mar 20, 2020
@ivanpauno
Copy link
Copy Markdown
Member

ivanpauno commented Mar 20, 2020

@ivanpauno @wjwwood The tests now pass. If someone could merge this for me, I would appreciate it.

Merged both this PR and the one in rclcpp.

@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Mar 20, 2020 via email

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

Reviewers

@wjwwood wjwwood wjwwood approved these changes

@ivanpauno ivanpauno ivanpauno approved these changes

Assignees

@brawner brawner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@brawner @dirk-thomas @ivanpauno @wjwwood