-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
jsoncons recipe #16256
jsoncons recipe #16256
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Overall this looks really good, there's a few small mistakes, you can use the template to help fix them https://github.com/conan-io/conan-center-index/blob/master/docs/package_templates/header_only/all/conanfile.py :)
good work so far
recipes/jsoncons/all/conanfile.py
Outdated
homepage = "https://github.com/danielaparker/jsoncons" | ||
url = "https://github.com/danielaparker/jsoncons" |
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.
Please run this locally with the hooks enabled 🙏
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hey @prince-chrismc ! |
prince-chrismc/conan-center-index-pending-review#1 You can see the graphs theres quiet a hefty backlog of PRs and we are working our way through them :) |
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.
Looks nice! Thanks for your patience while we processed all the backlog, we appreciate it :)
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 9 (
Conan v2 pipeline (informative, not required for merge) ✔️
All green in build 8 (
|
self.cpp_info.set_property("cmake_file_name", "jsoncons") | ||
self.cpp_info.set_property("cmake_target_name", "jsoncons") | ||
|
||
# TODO: to remove in conan v2 once cmake_find_package* generators removed | ||
self.cpp_info.names["cmake_find_package"] = "jsoncons" | ||
self.cpp_info.names["cmake_find_package_multi"] = "jsoncons" |
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.
cmake_find_package and cmake_find_package_multi are obviously broken here, no one noticed that?
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.
They shouldnt be used anyways :) Wasnt a blocker in my head
* jsoncons * Fixed imports * layout src_folder=src * Fixed get * Fixed build and other improvements * Added settings * More fixes
Specify library name and version: jsoncons/0.169.0
Adding Conan support to jsoncons. Agreed with the owner of the library in this issue.