-
Notifications
You must be signed in to change notification settings - Fork 451
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
Otlp http cmake build fail #1133
Otlp http cmake build fail #1133
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1133 +/- ##
=======================================
Coverage 93.39% 93.39%
=======================================
Files 165 165
Lines 6229 6229
=======================================
Hits 5817 5817
Misses 412 412 |
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. Just curious why build didn't fail in our CI env.
probably because of including |
@@ -21,14 +21,15 @@ | |||
#include "google/protobuf/stubs/common.h" | |||
|
|||
#if defined(GOOGLE_PROTOBUF_VERSION) && GOOGLE_PROTOBUF_VERSION >= 3007000 | |||
# include "google/protobuf/stubs/stringpiece.h" |
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.
Wondering why it is necessary to include stringpiece.h because Base64Escape
is declared in strutil.h
?
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.
yeah thanks, removed it.
CI is not failing probably because we are using a newer version of protobuf. The explicit declaration is only old protobuf versions. |
Fixes #1132 (issue)
Changes
Please provide a brief description of the changes here.
Use proper overload.
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes