-
Notifications
You must be signed in to change notification settings - Fork 161
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 out parameters for rcl_get_node_names and rcl_get_node_names_with_enclaves #1137
Conversation
Signed-off-by: Felix Penzlin <eeefix@gmail.com>
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.
Thanks for these improvements.
I've left a few wording changes.
Co-authored-by: Chris Lalancette <clalancette@gmail.com> Signed-off-by: Felix Penzlin <eeefix@gmail.com>
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.
Sorry, two more changes for the linters, then I'll run CI.
Co-authored-by: Chris Lalancette <clalancette@gmail.com> Signed-off-by: Felix Penzlin <eeefix@gmail.com>
@clalancette Regarding the (The allocator is not yet in use for rcl_get_node_names, but it might improve consistency to switch to the pointer type here as well.) |
This is an ongoing discussion; see #264 . There are tradeoffs between having the parameter be a pointer (where we only have to copy the pointer, but then have to check for NULL pointer in the callee), and having the parameter be a structure (where we have to copy the whole structure, but then don't have to check for NULL in the callee). We still haven't resolved it one way or the other, so I don't think we should get bogged down in it here. |
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.
lgtm with green CI
No description provided.