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

dylib: new recipe #20803

Merged
merged 6 commits into from
Oct 31, 2023
Merged

dylib: new recipe #20803

merged 6 commits into from
Oct 31, 2023

Conversation

martin-olivier
Copy link
Contributor

Specify library name and version: dylib/2.2.1

@CLAassistant
Copy link

CLAassistant commented Oct 28, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

Sign in to view
This was referenced Oct 28, 2023
Closed
Closed
@conan-center-bot

This comment has been minimized.

Sign in to view
@prince-chrismc prince-chrismc mentioned this pull request Oct 28, 2023
Open
@conan-center-bot

This comment has been minimized.

Sign in to view
uilianries
uilianries reviewed Oct 30, 2023
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 really good, only few comments. Thank you for your contribution!

recipes/dylib/all/conanfile.py Outdated Show resolved Hide resolved
recipes/dylib/all/test_package/CMakeLists.txt Show resolved Hide resolved
@uilianries uilianries self-assigned this Oct 30, 2023
@martin-olivier
Copy link
Contributor Author

Hello @uilianries,

Thanks for the review.
I still have forbidden access on the CI results :/

@uilianries
Copy link
Member

Hello @uilianries,

Thanks for the review.
I still have forbidden access on the CI results :/

@martin-olivier it's because you are merged in the allowed user list yet, but should be soon. After that, the ci will build your PR for real, then you will have the results available as GitHub comment.

@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
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 8 (50bfa6dc5f02d477026728978d1af43840d247cd):

  • dylib/2.2.1:
    All packages built successfully! (All logs)

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 8 (50bfa6dc5f02d477026728978d1af43840d247cd):

  • dylib/2.2.1:
    All packages built successfully! (All logs)

@martin-olivier
Copy link
Contributor Author

@uilianries

I have been added on the user list 👍
I have added the link with libdl on unix and provided the configuration flag for ctest to fix the CI

uilianries
uilianries approved these changes Oct 31, 2023
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. Thank for your pull request!

recipes/dylib/all/test_package/CMakeLists.txt
target_link_libraries(${PROJECT_NAME} PRIVATE dylib::dylib)
target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_11)

add_test(NAME ${PROJECT_NAME} COMMAND ${PROJECT_NAME})
Copy link
Member

Choose a reason for hiding this comment

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

Usually we dont use ctest because it hides the output, but is not a blocker, just consider for futures PRs.

@uilianries
Copy link
Member

@martin-olivier I approved your PR, but it still needs another review from the community/team. Please, take a look in https://github.com/conan-io/conan-center-index/blob/master/docs/review_process.md#rule-of-2-reviews to get more information about CCI review process.

@conan-center-bot conan-center-bot merged commit daec0d0 into conan-io:master Oct 31, 2023
5 checks passed
seppeon pushed a commit to seppeon/conan-center-index that referenced this pull request Oct 21, 2024
@martin-olivier @uilianries @seppeon
(conan-io#20803) dylib: new recipe
78e9d0a 
* dylib: new recipe

Signed-off-by: Martin Olivier <[email protected]>

* fix: support conan 1.X.X

Signed-off-by: Martin Olivier <[email protected]>

* fix: removed author section in conanfile

Co-authored-by: Uilian Ries <[email protected]>

* fix: added a newline at the end of each files

Signed-off-by: Martin Olivier <[email protected]>

* fix: added link with libdl on unix

* fix: providing configuration for tests

Signed-off-by: Martin Olivier <[email protected]>

---------

Signed-off-by: Martin Olivier <[email protected]>
Co-authored-by: Uilian Ries <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwillikers jwillikers jwillikers approved these changes

@uilianries uilianries uilianries approved these changes

Assignees

@uilianries uilianries

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@martin-olivier @CLAassistant @conan-center-bot @uilianries @jwillikers