-
Notifications
You must be signed in to change notification settings - Fork 494
[BUILD] Fixes compiling problems in NDK r27 #3517
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
Conversation
✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.
|
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.
Pull Request Overview
This PR addresses compilation issues in NDK r27 by reorganizing includes and correcting std::shared_ptr<FILE>
usage for file handles.
- Moved the
nlohmann/json.hpp
include to eliminate duplicates. - Replaced the invalid
std::make_shared<FILE>()
with a directstd::shared_ptr<FILE>
allocation usingfclose
as a deleter.
Comments suppressed due to low confidence (2)
exporters/otlp/src/otlp_file_client.cc:1296
- [nitpick] The variable name
of
is ambiguous; consider renaming it tofile_handle
orfile_ptr
for better clarity.
std::shared_ptr<FILE> of = std::shared_ptr<std::FILE>(new_file, fclose);
exporters/otlp/src/otlp_file_client.cc:1239
- Consider adding a unit test to verify that the file handle is correctly opened and closed under NDK r27 to prevent future regressions.
}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3517 +/- ##
==========================================
+ Coverage 89.95% 89.96% +0.02%
==========================================
Files 219 219
Lines 7051 7051
==========================================
+ Hits 6342 6343 +1
+ Misses 709 708 -1 🚀 New features to boost your workflow:
|
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
Commit owned by owent and not by copilot, so eacyCLA is clear here.
Ok to merge, with minor fix on std::FILE versus FILE.
@owent Thanks for the std fixes. Will merge once CI completes. |
Fixes #3516
Changes
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes