refactor rosidl_typesupport_introspection_cpp::typesupport_introspection_identifier into cpp file #12
Conversation
|
+1 |
| @@ -18,7 +18,7 @@ | |||
| namespace rosidl_typesupport_introspection_cpp | |||
| { | |||
|
|
|||
| static const char * typesupport_introspection_identifier = "introspection"; | |||
| static const char * typesupport_introspection_identifier; | |||
There was a problem hiding this comment.
I would say yes. Why should it not be static? static const is the common alternative to defines.
There was a problem hiding this comment.
Because you usually don't use static symbols from other libraries. Maybe it is necessary for how you are exporting the symbol from identifier.cpp to the libraries and executables which use the symbol, but I don't know. It seems to me that const is sufficient since the only thing static adds to this is setting the linkage to internal, which doesn't apply since it is a declaration and not an initialization. At any rate I think the question should not by "why shouldn't it be static" but rather "why should it be static"?
If it works as-is that's fine, I'd just like to understand the reason for it being that way.
dirk-thomas
force-pushed
the
refactor_typesupport
branch
6 times, most recently
from
2209f7d
to
679d183
Compare
…ion_identifier into cpp file
dirk-thomas
force-pushed
the
refactor_typesupport
branch
from
a184440
to
33f2c55
Compare
|
Finally ready for review. Windows slave was able to build this: http://54.183.26.131:8080/view/ros2/job/ros2_batch_ci_test_windows/15/ |
|
As a follow up of this PR each package using macros for visibility control must use its own macros and not reuse the macros from another package: ros2/ros2#37 |
|
+1 |
1 similar comment
|
+1 |
refactor rosidl_typesupport_introspection_cpp::typesupport_introspection_identifier into cpp file
Connects to ros2/ros2#30.