-
Notifications
You must be signed in to change notification settings - Fork 436
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
update style #445
update style #445
Conversation
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.
Some comments, but I'm good with the changes as-is, since they're an improvement.
typename std::allocator<void>::template rebind<T>::other>::value, | ||
std::default_delete<T>, | ||
AllocatorDeleter<Alloc> | ||
>::type; |
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.
This is better. It's unfortunate that the last line >::type
isn't indented one less (should end on the same column as the line that starts it).
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.
A similar comment applies to all follow multi-line templates.
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.
When targeting the latest release this "improvement" is sadly not possible. Therefore I revert this change.
rclcpp/include/rclcpp/exceptions.hpp
Outdated
@@ -116,7 +116,7 @@ throw_from_rcl_error( | |||
rcl_ret_t ret, | |||
const std::string & prefix = "", | |||
const rcl_error_state_t * error_state = nullptr, | |||
void (*reset_error)() = rcl_reset_error); | |||
void (* reset_error)() = rcl_reset_error); |
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.
Not sure this is better (more correct), but I'm fine with it too.
|
||
template<typename FunctorT, typename ... Args> | ||
struct check_arguments : std::is_same< | ||
typename function_traits<FunctorT>::arguments, | ||
std::tuple<Args ...> | ||
> | ||
> |
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.
This one has the right indentation for the closing >
, but does 4 spaces rather than two on the inside... Which is somehow the counterpart of the one I commented on above. If only we could combine the two...
04071cf
to
0ce598b
Compare
0ce598b
to
3907e16
Compare
Signed-off-by: Abby Xu <[email protected]>
Connect to
ament/uncrustify#20. Connect to ament/uncrustify#21.Ready for review.