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.

Sign up for GitHub

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

Jump to bottom

[json-schema-validator] Conan v2 support #13959

Merged
merged 9 commits into from
Nov 14, 2022
Merged

[json-schema-validator] Conan v2 support #13959

merged 9 commits into from
Nov 14, 2022

Conversation

garethsb
Copy link
Contributor

@garethsb garethsb commented Nov 2, 2022 •
edited
Loading

  • Conan v2 support
  • Bump nlohmann_json/3.11.2

@garethsb garethsb mentioned this pull request Nov 2, 2022
Closed
@github-actions github-actions bot mentioned this pull request Nov 2, 2022
Open
@conan-center-bot conan-center-bot added the Bump dependencies Only bumping dependencies versions in the recipe label Nov 2, 2022
@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2022

Hooks produced the following warnings for commit 1976f71
json-schema-validator/2.1.0 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libnlohmann_json_schema_validator.so' links to system library 'm' but it is not in cpp_info.system_libs.
json-schema-validator/2.0.0 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libjson-schema-validator.so' links to system library 'm' but it is not in cpp_info.system_libs.

@uilianries
Copy link
Member

The CI linter in not happy. This recipe is outdated and requires more update. Could you please update it? We can help you. Please, take a look on the annotations on https://github.com/conan-io/conan-center-index/pull/13959/files. Also, https://github.com/conan-io/conan-center-index/blob/master/docs/v2_linter.md is a good guide.

@conan-center-bot conan-center-bot added Service Under Maintenance and removed Bump dependencies Only bumping dependencies versions in the recipe labels Nov 7, 2022
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@garethsb
Copy link
Contributor Author

garethsb commented Nov 7, 2022

@uilianries I added the patches to fix the cmake_minimum_version problem and now the linter is complaining about missing patch_description and patch_type entries... The docs for conan.tools.files.apply_conandata_patches() mention them in an example but otherwise the spec seems to be "kwargs: Extra parameters that can be added and will contribute to output information" - certainly nothing to imply they are required, and no list of valid patch_type values...

@conan-center-bot

This comment has been minimized.

Sign in to view
uilianries
uilianries reviewed Nov 10, 2022
View reviewed changes
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, need some adjusts

recipes/json-schema-validator/all/conanfile.py Outdated Show resolved Hide resolved
recipes/json-schema-validator/all/conanfile.py Outdated Show resolved Hide resolved
recipes/json-schema-validator/all/conanfile.py Outdated Show resolved Hide resolved
recipes/json-schema-validator/all/conanfile.py Outdated Show resolved Hide resolved
recipes/json-schema-validator/all/conanfile.py Outdated Show resolved Hide resolved
recipes/json-schema-validator/all/conanfile.py Outdated Show resolved Hide resolved
recipes/json-schema-validator/all/conanfile.py Outdated Show resolved Hide resolved
recipes/json-schema-validator/all/conanfile.py Outdated Show resolved Hide resolved
recipes/json-schema-validator/all/conanfile.py Outdated Show resolved Hide resolved
@uilianries
Copy link
Member

@uilianries I added the patches to fix the cmake_minimum_version problem and now the linter is complaining about missing patch_description and patch_type entries... The docs for conan.tools.files.apply_conandata_patches()

@garethsb It's not mandatory, but very welcome. Patches need to be described to help other users to understand their nature. More info: https://github.com/conan-io/conan-center-index/blob/master/docs/conandata_yml_format.md#patches-fields

@garethsb
Copy link
Contributor Author

More info: https://github.com/conan-io/conan-center-index/blob/master/docs/conandata_yml_format.md#patches-fields

@uilianries That's the docs I was looking for, thanks! I forget sometimes it's conan docs I want and sometimes CCI docs...

@garethsb @uilianries
Apply suggestions from code review
39c87ae 
Co-authored-by: Uilian Ries <uilianries@gmail.com>
@uilianries
Copy link
Member

More info: https://github.com/conan-io/conan-center-index/blob/master/docs/conandata_yml_format.md#patches-fields

@uilianries That's the docs I was looking for, thanks! I forget sometimes it's conan docs I want and sometimes CCI docs...

Everything is there, sometimes is hard to find or because the search engine is naive or because is more hidden than it should. In case you find some difficult to find anything there, please, open an issue to docs. For me and other devs, we used to read that everyday so we forget how hard can be finding some useful information there.

prince-chrismc
prince-chrismc previously approved these changes Nov 10, 2022
View reviewed changes
@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

Hooks produced the following warnings for commit 39c87ae
json-schema-validator/2.1.0 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libnlohmann_json_schema_validator.so' links to system library 'm' but it is not in cpp_info.system_libs.
json-schema-validator/2.0.0 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libjson-schema-validator.so' links to system library 'm' but it is not in cpp_info.system_libs.

@garethsb garethsb mentioned this pull request Nov 11, 2022
Closed
4 tasks
@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

Hooks produced the following warnings for commit 11c99e0
json-schema-validator/2.1.0 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libnlohmann_json_schema_validator.so' links to system library 'm' but it is not in cpp_info.system_libs.
json-schema-validator/2.0.0 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libjson-schema-validator.so' links to system library 'm' but it is not in cpp_info.system_libs.

uilianries
uilianries previously approved these changes Nov 11, 2022
View reviewed changes
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@garethsb
Copy link
Contributor Author

@uilianries Do we need to upgrade the test_package to use CMakeDeps too? And do we have to keep the existing one as test_v1_package?

@conan-center-bot

This comment has been minimized.

Sign in to view
@garethsb garethsb changed the title [json-schema-validator] Bump nlohmann_json/3.11.2 [json-schema-validator] Conan v2 support Nov 14, 2022
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline

All green in build 11 (ff671dcf26ffcba589b68cf0ddaf9839bd3564a1):

  • json-schema-validator/2.0.0@:
    All packages built successfully! (All logs)

  • json-schema-validator/2.1.0@:
    All packages built successfully! (All logs)

@uilianries
Copy link
Member

@uilianries Do we need to upgrade the test_package to use CMakeDeps too? And do we have to keep the existing one as test_v1_package?

Yes,, please.

uilianries
uilianries approved these changes Nov 14, 2022
View reviewed changes
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@conan-center-bot conan-center-bot merged commit 97175e5 into conan-io:master Nov 14, 2022
SpaceIm
SpaceIm reviewed Nov 14, 2022
View reviewed changes
recipes/json-schema-validator/all/conanfile.py

def package_info(self):
self.cpp_info.set_property("cmake_file_name", "nlohmann_json_schema_validator")
self.cpp_info.set_property("cmake_target_name", "nlohmann_json_schema_validator")
self.cpp_info.libs = ["json-schema-validator" if tools.Version(self.version) < "2.1.0" else "nlohmann_json_schema_validator"]
self.cpp_info.set_property("cmake_target_name", "nlohmann_json_schema_validator::nlohmann_json_schema_validator")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a regression, target name was correct and aligned with upstream imported target.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing.

@SpaceIm SpaceIm mentioned this pull request Nov 14, 2022
Merged
4 tasks
@garethsb garethsb deleted the patch-1 branch November 14, 2022 23:33
@garethsb garethsb mentioned this pull request Nov 23, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SpaceIm SpaceIm SpaceIm left review comments

@prince-chrismc prince-chrismc prince-chrismc approved these changes

@uilianries uilianries uilianries approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@garethsb @conan-center-bot @uilianries @prince-chrismc @SpaceIm