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

Make vcpkg.json comment multiline #5175

Merged
merged 2 commits into from
Nov 22, 2023

Conversation

antkmsft
Copy link
Member

@antkmsft antkmsft commented Nov 21, 2023

Now that microsoft/vcpkg#34877 is fixed, we can format vcpkg.json comment as multi-line.
This way, it is better visible, and it aligns with the existing portfile.cmake comment, which is also multi-line:

# NOTE: All changes made to this file will get overwritten by the next port release.
# Please contribute your changes to https://github.com/Azure/azure-sdk-for-cpp.

I am updating the vcpkg commit SHA to microsoft/vcpkg@43cf47e, because that is the commit which updates vcpkg to the 2023-11-16 release - vcpkg-tool release which contains bugfix for microsoft/vcpkg#34877: https://github.com/microsoft/vcpkg-tool/releases/tag/2023-11-16

--

@LarryOsterman, I am adding #include <opentelemetry/sdk/trace/recordable.h> for the sdk/core/azure-core-tracing-opentelemetry/test/ut/test_exporter.hpp. It only updates the unit test code and does not require a package release (unit tests are not compiled when our SDK is being installed as vcpkg package). It is not a breaking change either. Everything should compile with both the previous version of opentelemetry-cpp, and the new one.

The reason is - vcpkg now has an updated version of opentelemetry-cpp - 1.12.0 - microsoft/vcpkg#33983.

And one of the commits which went into that version, was "[SDK] Header files cleanup, use forward declarations", which has specifically dropped #include <opentelemetry/sdk/trace/recordable.h> line from the opentelemetry/sdk/trace/exporter.h, which is the only header file that we were including before at that place. But it is no longer sufficient:
open-telemetry/opentelemetry-cpp@cfcda57#diff-c44d1944597823ab10bd389630f778367fb19eaac5ae68cdefa5e8d5f71a5783L9

@antkmsft antkmsft added the EngSys This issue is impacting the engineering system. label Nov 21, 2023
@antkmsft antkmsft merged commit 05d1f54 into Azure:main Nov 22, 2023
300 checks passed
@antkmsft antkmsft deleted the vcpkg-json-multiline-comment branch November 22, 2023 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EngSys This issue is impacting the engineering system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants