-
Notifications
You must be signed in to change notification settings - Fork 35
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
Remove rcl_logging_log4cxx. #78
Conversation
We don't ever use it, and having it around causes a few problems: 1. We have an unnecessary, and unneeded dependency on log4cxx in the core. 2. We can't currently build against libc++ with log4cxx. 3. Log4cxx causes some absolute paths to be in the delivered packages on Windows. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
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.
Sounds fine to me
The other good thing about doing this is that we'll be able to get rid of the log4cxx dependency. Before I go ahead with this, any objections @cottsay or @nuclearsandwich ? |
Even though the full CI was a month ago, nothing substantial has changed here. So I'm just going to rely on that and the approval from @ivanpauno, and go ahead and merge this. There is probably some follow-up work in the documentation and the CI jobs that needs to be done, which I'll look at next. |
It is no longer required as of ros2/rcl_logging#78 Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
It is no longer required as of ros2/rcl_logging#78 Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
We don't ever use it, and having it around causes a few problems:
core.
on Windows.
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
@ivanpauno FYI, this should get rid of one more of the problems pointed out in ros2/rclcpp#1688