Skip to content
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

Clarify what not_compsable.cpp means #224

Closed
wants to merge 1 commit into from

Conversation

davetcoleman
Copy link

From discussion: #160

@tfoote tfoote added the in review Waiting for review (Kanban column) label Jan 28, 2019

"Composable" refers to the ability for a node to be used as part of a Composition (running multiple nodes in a single process).

Note that not_composable.cpp instantiates a rclcpp::Node without subclassing it. This was the typical usage model in ROS 1, but unfortunately this style of coding is not compatible with composing multiple nodes into a single process. Thus, it is no longer the recommended style for ROS 2.
Copy link
Member

Choose a reason for hiding this comment

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

Please start each sentence on a newline (as described in the developer guide).

Please remove the phrase unfortunately from the text. This decision is not about preference but about necessity.

Copy link
Author

Choose a reason for hiding this comment

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

I just copied this paragraph from the subscriber README also located in this repo:

https://github.com/ros2/examples/tree/master/rclcpp/minimal_subscriber

It already had the nit-pick issues you are asking me to now fix. I'm doing my best to improve documentation for ROS2 but I don't have the motivation to iterate on this.

Copy link
Member

Choose a reason for hiding this comment

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

I just copied this paragraph from the subscriber README also located in this repo:

Actually that is only partially correct. You copied the rendered output but not the actual source. The source you tried to copy does follow the developer guide as well as uses markup to format specific parts of the sentence: https://raw.githubusercontent.com/ros2/examples/af08e6f7ac50f7808dbe6165f1adfd8e6cd3a79c/rclcpp/minimal_subscriber/README.md

The phrase unfortunately is certainly a personal opinion but since you closed the PR I won't bother about it either.

@tfoote tfoote removed the in review Waiting for review (Kanban column) label Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants