Conversation
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
|
clalancette
left a comment
clalancette
left a comment
There was a problem hiding this comment.
This is fantastic, and exactly what we need. Thanks for doing this!
I have two nits inline. Also, I'm wondering if we should just move the implementation of the constructor that takes a std::string into the C++ file as well. It is minor, but putting it there helps on compile times, and just makes it so all of the implementation is in one place.
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Moved all method and free function definitions to the source file in 82a0901. |
clalancette
left a comment
clalancette
left a comment
There was a problem hiding this comment.
Looks good with green CI.
|
CI (build everything, test rcpputils):
|
|
Same jobs also including ros2/rosbag2#600:
|
|
Windows once again, also testing ros2/rcl_logging#71:
|
2 participants
windows.hshould not be included in any header file because it bizarrely pollutes the global namespace with macros that can easily collide with other names.My favorites: min, max, ERROR, NO_ERROR, and even more.
IMO trying to make all our headers compatible with
windows.his a bit hopeless, so we should avoid includingwindows.hin header files and use custom workarounds in source files when appropriate (an#undefor#define NOMINMAXusually fix collisions).