-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
prometheus-cpp: add version 1.2.4, require C++11 only #22079
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
Regenerating missing binaries now |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a change regarding the minimal C++ standard required to build this project, it's C++11 again.
jupp0r/prometheus-cpp@v1.1.0...v1.2.3
I would suggest trying to build everything with C++11 directly, excluding the C++14 check.
@uilianries |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@@ -36,15 +35,19 @@ class PrometheusCppConan(ConanFile): | |||
|
|||
@property | |||
def _min_cppstd(self): | |||
return 11 if Version(self.version) < "1.1.0" else 14 | |||
return "14" if Version(self.version) != "1.1.0" else "11" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return "14" if Version(self.version) != "1.1.0" else "11" | |
return "14" if Version(self.version) >= "1.1.0" else "11" |
Includes 1.1.0 and higher to C++14
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you mentioned, prometheus-cpp/1.2.4 requires only C++11 again.
So prometheus-cpp/1.1.0 is the only version to require C++14.
I realized the above code is wrong condition, I already fixed it.
@@ -5,8 +5,8 @@ find_package(prometheus-cpp CONFIG REQUIRED) | |||
|
|||
add_executable(${PROJECT_NAME} test_package.cpp) | |||
target_link_libraries(${PROJECT_NAME} PRIVATE prometheus-cpp::push prometheus-cpp::pull) | |||
if(${prometheus-cpp_VERSION} VERSION_LESS "1.1.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could simplify all these line to target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_14)
only. C++14 is compatible with C++11, so it should not break the test package, so we don't need this condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe cxx_std_11
is required in all versions except 1.1.0.
To simplify this code, should I use target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_11)
in all versions?
Conan v1 pipeline ✔️All green in build 8 (
Conan v2 pipeline ✔️
All green in build 8 (
|
Specify library name and version: prometheus-cpp/*