Skip to content

Conversation

maztheman
Copy link

Other minor fixes as well

@dgoffredo
Copy link
Contributor

Hi, @maztheman. Thank you for these contributions. CMake is my least favorite part of any C++ project :)

Could you please explain these changes, your reasons for proposing them, and your use case with the library? That will help with the review.

@maztheman
Copy link
Author

Main use case is to fix some of the errors you get when you try to FetchContent this repo (using cmake). one of the biggest issues is how you append the cmake module. After that the using CPM part is just for easier understanding of the cmake files. I noticed this project "saves" some properties before compiling via git modules. I felt there could be a better way of writing the cmake files.

@maztheman
Copy link
Author

oh, and as you are probably aware the opentracing library is deprecated, and such I am trying to put in opentelemetry with the shim in place.

@dgoffredo
Copy link
Contributor

oh, and as you are probably aware the opentracing library is deprecated, and such I am trying to put in opentelemetry with the shim in place.

I'm working on an opentelemetry::trace::TracerProvider implementation that will use dd-trace-cpp under the hood.

If you're comfortable writing your code in terms of a Datadog-only API, then you could use dd-trace-cpp directly today. If not, you'll have to continue using this library with OpenTelemetry's OpenTracing shim.

Main use case is to fix some of the errors you get when you try to FetchContent this repo (using cmake). one of the biggest issues is how you append the cmake module. After that the using CPM part is just for easier understanding of the cmake files. I noticed this project "saves" some properties before compiling via git modules. I felt there could be a better way of writing the cmake files.

I'll take a closer look at your changes and share them with my teammates. Thanks again for the contribution.

@maztheman
Copy link
Author

Looks like these changes might have to wait until the integration_test_nginx issue can be solved. Apparently CPM requires 3.14 but the integration_test_nginx only has cmake 3.13.

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.

2 participants