-
Notifications
You must be signed in to change notification settings - Fork 438
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
[BUILD] Remove defining NOMINMAX from api #2420
[BUILD] Remove defining NOMINMAX from api #2420
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2420 +/- ##
==========================================
- Coverage 87.06% 87.04% -0.01%
==========================================
Files 199 199
Lines 6079 6079
==========================================
- Hits 5292 5291 -1
- Misses 787 788 +1
|
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.
For the changes in API, totally agree, this is the most robust thing to do.
For the changes in exporters and SDK, I think the changes are not really needed, especially in *.cc
because the Windows.h
header file is not used and will not cause conflicts, but ok to have the extra parenthesis to have a consistent code.
Even when not needed, this will prevent two different styles to proliferate in the code base with copy and paste.
Approved, thanks for the fix.
Currently, including OpenTelemetry API requires NOMINMAX to be defined for Windows user which excludes the min/max macro from the Windows header files. There are cases when both macros are used by the customer code so requiring NOMINMAX will cause incompatibility. This PR wraps the usage of min/max from C++ standard library with parenthesis. This can avoid the naming conflict with the macro names min/max.
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes