Expose native handles #243
Conversation
dirk-thomas
force-pushed
the
expose_native_handles
branch
from
de2ffbe
to
c894c00
Compare
|
|
||
| extern "C" | ||
| { |
There was a problem hiding this comment.
does it hurt to keep it ? otherwise if we ever add functions here we will have to remember to add back the extern
There was a problem hiding this comment.
I don't see a reason to declare the symbol extern "C". Also none of the other RMW impl. does that.
rmw_connext_cpp/src/rmw_client.cpp
Outdated
| #include "process_topic_and_service_names.hpp" | ||
| #include "type_support_common.hpp" | ||
| #include "types/connext_static_client_info.hpp" | ||
| #include "rmw_connext_cpp/connext_static_client_info.hpp" |
There was a problem hiding this comment.
Nit: include order (same in other files below)
There was a problem hiding this comment.
I fixed the order of this include but as far as I see there are many more includes not following that rule since it is not enforced by the linter atm.
There was a problem hiding this comment.
Agreed, I made the comment because your analog PR in rmw_fastrtps was complying and not this one. Apparently you fixed only this one though and not the other ones in this PR
dirk-thomas
force-pushed
the
expose_native_handles
branch
from
c894c00
to
75039bb
Compare
Similar to ros2/rmw_fastrtps#145 and ros2/rmw_fastrtps#146.