Skip to content
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] Fix removing of NOMINMAX on Windows #2449

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

ThomsonTan
Copy link
Contributor

@ThomsonTan ThomsonTan commented Dec 13, 2023

Changes

NOMINMAX was removed in our API in below PR, but one more fix needed for a call of ::max in the API. It was missed because this line is only hit when building for C++ 17 and above.

#2420

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@ThomsonTan ThomsonTan requested a review from a team December 13, 2023 19:28
Copy link

codecov bot commented Dec 13, 2023

Codecov Report

Merging #2449 (b166a11) into main (96e5078) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2449   +/-   ##
=======================================
  Coverage   87.06%   87.06%           
=======================================
  Files         199      199           
  Lines        6087     6087           
=======================================
  Hits         5299     5299           
  Misses        788      788           

@marcalff marcalff changed the title Fix removing of NOMINMAX on Windows [BUILD] Fix removing of NOMINMAX on Windows Dec 13, 2023
@marcalff marcalff merged commit 0b9371d into open-telemetry:main Dec 13, 2023
45 checks passed
waywardmonkeys added a commit to waywardmonkeys/vcpkg that referenced this pull request Jan 4, 2024
The patch to add a dependency of opentelemetry-proto on gRPC should
not be needed after the upstream changes in:

open-telemetry/opentelemetry-cpp#2268

Additionally, the "zpages" feature is removed as it is no longer
present upstream after having been deprecated in a previous release.

A patch is added extracted from open-telemetry/opentelemetry-cpp#2475
and open-telemetry/opentelemetry-cpp#2449
which fix problems that arose after `NOMINMAX` was no longer
defined within opentelemetry-cpp on Windows.

Fixes microsoft#35992.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants