opentelemetry-cpp
opentelemetry-cpp copied to clipboard
Remove unnecessary usages of nostd::type_traits in API.
The API package already uses std in other places.
Question for maintainers, what do you prefer:
- Still keep nostd type_traits and always make them equivalent with std since we are on c++14.
- Remove nostd::type_traits form the API.
- Doing nothing.
Option 1 looks good to me, as there could be an inevitable scenario where users would have taken a dependency on nostd::type_traits.
Some old toolchains do not support C++14 libraries completely. Could we just keep the nostd version here?
But C++ 14 is required for OpenTelemetry. Or which version of toolchain is still under concern?
https://github.com/open-telemetry/opentelemetry-cpp#supported-c-versions
Question for maintainers, what do you prefer:
1. Still keep nostd type_traits and always make them equivalent with std since we are on c++14. 2. Remove nostd::type_traits form the API. 3. Doing nothing.
Given that opentelemetry-cpp now officially does not support C++11, and requires C++14, the logical answer should be 2, and keeping a typedef from nostd::type_traits for std::type_traits to avoid breaking users who used the nostd flavor.
And I even suggested that myself (to remove nostd::type_traits) in a different PR.
There is more to this story however.
C++14 is officially supported, which means we aim to support any combination of API, SDK, exporters, libraries (GTest, benchmark, CURL, grpc, ...) with C++14, and want to maintain a working CI for this.
At the same time, C++11 is informally supported.
opentelemetry-cpp is still used, even with current versions, with C++11, with some well chosen combinations of exporters and third party libraries, and one known user still on C++11 is @owent
While we do not want to maintain a full CI for C++11, we (opentelemetry-cpp) should also attempt, if possible, to maintain a status quo and still have code that works in C++11, as a courtesy for a major contributor: @owent ranks 5th in terms of commits (82), and 2nd in terms of lines added (34,872), at time of writing.
Personally, I would love to see nostd::type_traits go away, but it looks we will have to keep some nostd parts a little longer, until the time when @owent no longer has the C++11 constraint.
So, in my opinion, option "3 - doing nothing ... for now" is the only valid choice.
Question for maintainers, what do you prefer:
1. Still keep nostd type_traits and always make them equivalent with std since we are on c++14. 2. Remove nostd::type_traits form the API. 3. Doing nothing.Given that opentelemetry-cpp now officially does not support C++11, and requires C++14, the logical answer should be 2, and keeping a typedef from
nostd::type_traitsforstd::type_traitsto avoid breaking users who used the nostd flavor.And I even suggested that myself (to remove nostd::type_traits) in a different PR.
There is more to this story however.
C++14 is officially supported, which means we aim to support any combination of API, SDK, exporters, libraries (GTest, benchmark, CURL, grpc, ...) with C++14, and want to maintain a working CI for this.
At the same time, C++11 is informally supported.
opentelemetry-cpp is still used, even with current versions, with C++11, with some well chosen combinations of exporters and third party libraries, and one known user still on C++11 is @owent
While we do not want to maintain a full CI for C++11, we (opentelemetry-cpp) should also attempt, if possible, to maintain a status quo and still have code that works in C++11, as a courtesy for a major contributor: @owent ranks 5th in terms of commits (82), and 2nd in terms of lines added (34,872), at time of writing.
Personally, I would love to see
nostd::type_traitsgo away, but it looks we will have to keep somenostdparts a little longer, until the time when @owent no longer has the C++11 constraint.So, in my opinion, option "3 - doing nothing ... for now" is the only valid choice.
It's just fine to remove nostd::type_traits. What I want to do is use a small patch file to support C++11. I'm pushing teams to use new toolchains but it still need some time for told project to migrate.
I have another idea, can I add a compatible header to use nostd::type_traits to replace the type_traits in std namespace when using these old toolchains? It will leave the codes clean and just add a include sentense for each file include <type_traits>.
BTW: Another useful API only in C++14 is std::make_unique. I can also add it to this header.
As commented previously:
- opentelemetry-cpp is officially supported for C++14 and up
- some subset of opentelemetry-cpp is de facto still functional with C++11
There is some ambiguity between "supported in C++14" and "requiring C++14", and we would like to avoid actually requiring C++14, at least for some time, unless absolutely necessary.
Replacing nostd type traits with std type traits would require C++14 for no other gains.
This cleanup is valid, but postponed for now.
Closing.
To re evaluate when @owent no longer needs the code to still build with C++11.