-
Notifications
You must be signed in to change notification settings - Fork 76
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
Document names of frontend action arguments #409
base: rolling
Are you sure you want to change the base?
Document names of frontend action arguments #409
Conversation
Signed-off-by: Christophe Bedard <[email protected]>
This is a draft PR because this is a WIP just to get feedback on the execution/solution. |
* - ``parameters`` | ||
- ``param`` (``name``, ``value``) | ||
* - ``remappings`` | ||
- ``remap`` (``from``, ``to``) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For arguments like this, it's a bit more complicated than just providing these names. What people really need are examples. I'm tempted to link to each action's frontend tests (at least for the ones that have frontend tests in test_launch_ros
), like this one for the Node
action: https://github.com/ros2/launch_ros/blob/38152b2d04c6b6f126d31726ac9e12888b2e2e1f/test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py. However, I'm not sure how to do it properly (i.e., how to get a distro-specific link to the source code, or maybe the test file itself can be included in the docs?), and this could get outdated + it's stereotypically "bad," so I'd just overall like to get some feedback.
Any feedback here? The more I think about it, the more I think we should just document the names of the frontend arguments in the Python class documentation. Then it just shows up in the API docs, and it's extremely close to where the argument names are defined (in the same class). It feels weird to expect users to look at the Python action class docs to find the names of the frontend arguments, but it works, and, since that's the only documentation we have, people might just look there anyway. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i say this is nice enhancement for user-experience, especially yaml and xml users to launch the node. and i also agree on adding examples to https://docs.ros.org/en/rolling/How-To-Guides/Launch-file-different-formats.html, this would be the 1st place for user to visit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I agree with the sentiment that examples would also be good.
That said, I'm fine with adding in additional documentation. I especially like that this page is sort of at the "front" of the generated API documentation, so easy to find.
One thought here is how the individual actions are documented. It does seem like this is a bit of duplication of the pydoc strings that are in the class. Is it possible/would it make sense to have this "front page" documentation link to deeper into the generated API documentation? That way it will all be in sync.
Adding examples to the ROS 2 docs is a good idea.
so I'd just list some actions on this page and link to their API doc page? The frontend argument names would have to be documented in the Python class or constructor docstring (because currently they're not documented), but yes I agree, this would keep the actual documentation in the source code itself. |
When you say that you tested this with rosdoc2, what exactly do you mean? That is, where did you obtain rosdoc2, and how exactly did you run rosdoc2. I'd like to make recommendations on how this could work better with rosdoc2 both present and future, but that is challenging because rosdoc2 is in flux. |
By "tested with rosdoc2," I meant that the two simple pages I added show up in the generated docs. I ignored the rest. It's been a while, but:
|
Resolves #404
This adds boilerplate for
launch_ros
docs along with a page to document the argument names for the frontend actions. The setup is similar to this page in thelaunch
docs:Tested with
rosdoc2
.