[RSDK-13421] Improve C++ code quality: constexpr, consistent logging, namespace cleanup #39
Conversation
seanavery
changed the title
seanavery
changed the title
|
removing myself as a blocking review since I won't be able to approve next week |
| if (!pipeline) { | ||
| std::cerr << "Failed to create the pipeline" << std::endl; | ||
| g_print("Error: %s\n", error->message); | ||
| VIAM_SDK_LOG(error) << "Failed to create the pipeline: " << error->message; |
There was a problem hiding this comment.
Can VIAM_SDK_LOG be replaced with VIAM_RESOURCE_LOG to get more precise logs on which specific resource is emitting the logs?
There was a problem hiding this comment.
Sure thing. I think it makes sense to do this in a follow up as I will have to update across the codebase -- created ticket.
utils.cpp
Outdated
There was a problem hiding this comment.
Can this be replaced by a std::string?
There was a problem hiding this comment.
Switched to std::string. Still need a temporary const char* from std::getenv since it can return nullptr (which would crash if passed directly to std::string constructor)
utils.cpp
Outdated
There was a problem hiding this comment.
Can we change this to shortcircuit the bad case?, and also, if we use a string here instead of a pointer, we would be checking for it being empty.
There was a problem hiding this comment.
Switched to std::string and checking .empty(). Kept the same control flow since we need to fall through to device file detection when there's no env override.
4 participants
Description
Quick clean up C++ best practices:
pollution
Testing
Build and tests passing
Review
No change in functionality, 1 approval