-
Notifications
You must be signed in to change notification settings - Fork 696
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
refactor(planning_test_utils): separate hpp and cpp #7082
refactor(planning_test_utils): separate hpp and cpp #7082
Conversation
Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
template <typename T> | ||
void publishToTargetNode( | ||
rclcpp::Node::SharedPtr test_node, rclcpp::Node::SharedPtr target_node, std::string topic_name, | ||
typename rclcpp::Publisher<T>::SharedPtr publisher, T data, const int repeat_count = 3) | ||
{ | ||
if (topic_name.empty()) { | ||
int status; | ||
int status = 1; |
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.
very small question for my future code,
What's the motivation for this?
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.
It's always a good practice to initialize value.
* @param center_line_resolution The resolution for the centerline. | ||
* @return A HADMapBin message containing the map data. | ||
*/ | ||
HADMapBin make_map_bin_msg( |
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.
Do you have any distinguish between snake case and camel case?
(I know snake case is recommended for function name though
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.
@kyoichi-sugahara
For old function, I keep it as camelCase to avoid having to change other code. For new function, I used snake_case.
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!
Thank you for your work!
84e33d8
into
autowarefoundation:main
…on#7082) Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
…on#7082) Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Muhammad Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Description
Separate the inplementation in the header file to cpp file.
Added doxygen comments
Tests performed
Not applicable.
Effects on system behavior
Not applicable.
Interface changes
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.