Skip to content

[EXPORTER and SDK] Additional fixes after NOMINMAX removal on Windows#2475

Merged
lalitb merged 3 commits intoopen-telemetry:mainfrom
meastp:meastp-fix-nominmax
Jan 3, 2024
Merged

[EXPORTER and SDK] Additional fixes after NOMINMAX removal on Windows#2475
lalitb merged 3 commits intoopen-telemetry:mainfrom
meastp:meastp-fix-nominmax

Conversation

@meastp
Copy link
Copy Markdown
Contributor

@meastp meastp commented Jan 3, 2024

Fixes build failures after removal of NOMINMAX - previous commits from @ThomsonTan didn't have full coverage.

@meastp meastp requested a review from a team January 3, 2024 14:18
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Jan 3, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Copy Markdown
Member

@owent owent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, and LGTM after CLA signed.

Copy link
Copy Markdown
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once CLA is signed.

@meastp meastp force-pushed the meastp-fix-nominmax branch from 052d266 to f02636d Compare January 3, 2024 20:32
@meastp
Copy link
Copy Markdown
Contributor Author

meastp commented Jan 3, 2024

I modified my email, CLA is signed. Should be ready to merge :)

Thanks, @ThomsonTan @lalitb @owent

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ddfafff) 87.04% compared to head (f02636d) 87.06%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2475      +/-   ##
==========================================
+ Coverage   87.04%   87.06%   +0.02%     
==========================================
  Files         199      199              
  Lines        6087     6087              
==========================================
+ Hits         5298     5299       +1     
+ Misses        789      788       -1     

see 1 file with indirect coverage changes

@lalitb lalitb merged commit 0134bbe into open-telemetry:main Jan 3, 2024
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
which fixes problems that arose after `NOMINMAX` was no longer
defined within opentelemetry-cpp on Windows.

Fixes microsoft#35992.
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.

4 participants