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.

Sign up for GitHub

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
Closed

Clarify what not_compsable.cpp means #224

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion rclcpp/minimal_action_server/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# Minimal action server cookbook recipes

This package contains a few examples which show how to create action servers.
This package contains an example which shows how to create an action server.

"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.