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

Jump to bottom

added notes on prefix #399

Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from
Open

added notes on prefix #399

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
1 change: 1 addition & 0 deletions launch_ros/launch_ros/actions/node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ def __init__(
:param: remappings ordered list of 'to' and 'from' string pairs to be
passed to the node as ROS remapping rules
:param: ros_arguments list of ROS arguments for the node
:param: prefix command placed in front of executable on launch
Copy link
Member

Choose a reason for hiding this comment

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

We talked about this in the issue triage meeting, and the conclusion was that we should reference the base class that actually handles this argument, e.g. have a look at how Python does this for the subprocess API: https://docs.python.org/3/library/subprocess.html#subprocess.run

It's fine to give a brief description of the argument, but it should be separate from the other (actual) arguments and it should reference the place where it's actually documented, e.g. here: https://github.com/ros2/launch/blob/651f57451e4d0e93d055669764fcd588e2d9b1e6/launch/launch/actions/execute_process.py#L211-L216

It is indeed helpful to connect the dots here, but we don't want to duplicate too much documentation and risk them getting out of sync.

:param: arguments list of extra arguments for the node
"""
if package is not None:
Expand Down