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

jsoncons recipe #16256

Merged
merged 8 commits into from
Mar 27, 2023
Merged

jsoncons recipe #16256

merged 8 commits into from
Mar 27, 2023

Conversation

IronTony-Stark
Copy link
Contributor

Specify library name and version: jsoncons/0.169.0

Adding Conan support to jsoncons. Agreed with the owner of the library in this issue.


@CLAassistant
Copy link

CLAassistant commented Feb 23, 2023

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

Copy link
Contributor

@prince-chrismc prince-chrismc left a 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

Comment on lines 13 to 14
homepage = "https://github.com/danielaparker/jsoncons"
url = "https://github.com/danielaparker/jsoncons"
Copy link
Contributor

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 🙏

recipes/jsoncons/all/conanfile.py Outdated Show resolved Hide resolved
recipes/jsoncons/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@IronTony-Stark
Copy link
Contributor Author

Hey @prince-chrismc !
Thanks for the review and sorry for taking so long to respond.
I made some changes, now the pipelines are passing and everything should be in order.
Could you take a look at this PR again when you have the time please?

@prince-chrismc
Copy link
Contributor

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 :)

Copy link
Member

@AbrilRBS AbrilRBS left a 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 :)

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 9 (30ce2856431c39aab5cf62a485098739e2efa486):

  • jsoncons/0.169.0@:
    All packages built successfully! (All logs)

Conan v2 pipeline (informative, not required for merge) ✔️

Note: Conan v2 builds are informative and they are not required for the PR to be merged.

All green in build 8 (30ce2856431c39aab5cf62a485098739e2efa486):

  • jsoncons/0.169.0@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 0de381f into conan-io:master Mar 27, 2023
Comment on lines +42 to +47
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"
Copy link
Contributor

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?

Copy link
Contributor

@prince-chrismc prince-chrismc Mar 27, 2023

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

0xFireWolf pushed a commit to 0xFireWolf/conan-center-index that referenced this pull request Apr 2, 2023
* jsoncons

* Fixed imports

* layout src_folder=src

* Fixed get

* Fixed build and other improvements

* Added settings

* More fixes
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.

6 participants