-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix building with -DBUILD_EASYLOGGINGPP=OFF #11962
Conversation
.github/workflows/buildsCI.yaml
Outdated
@@ -21,7 +21,7 @@ jobs: | |||
|
|||
|
|||
#-------------------------------------------------------------------------------- | |||
Win_SH_EX_CfU: # Windows, shared, with Examples & Tools, and Check for Updates | |||
Win_SH_EX_CfU: # Windows, shared, with Examples & Tools, no EasyLogging and Check for Updates |
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.
Don't we also need to check on Linux?
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.
Since it's at the application level I thought it's enough to test on 1 OS, you think Linux is needed too?
We can add..
Why did we also change terminal-parser? |
When you added #include <rsutils/string/from.h> you removed the iostream include. It worked because easylogging include it somewhere, without it it was missing and we don't check it so you missed it. I checked and the cout was redundant, so instead of reverting the removal of include iostream, I removed the cout. |
How is it redundant? I don't see other code that outputs it... |
7ce9ec6
to
1c5167d
Compare
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.
LGTM
Fix #11905
Tracked on [LRS-829]