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

New package: libq #22828

Merged
merged 29 commits into from
Nov 15, 2024
Merged

New package: libq #22828

merged 29 commits into from
Nov 15, 2024

Conversation

metalMajor
Copy link
Contributor

@metalMajor metalMajor commented Feb 20, 2024

Specify library name and version: libq/20180504

I am not the creator of the library, but I am using this library for a while now, mostly in combination with libcurl to interact with a backend webserver, and wanted to hopefully promote some interest in the library by providing this package. The original author does not update it anymore it seems, but it's quite stable. Because the author never really officially released a 1.0, but mentions that the api would not change anymore before 1.0, I have used the date of the commit from which I created this package instead of 1.0.


@CLAassistant
Copy link

CLAassistant commented Feb 20, 2024

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.

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.

Hi! Thanks a lot for taking the time to contribute the recipe, we really appreciate it!

I've left some comments on the minor changes, but I'd suggest looking into the CMake package template https://github.com/conan-io/conan-center-index/tree/master/docs/package_templates/cmake_package - basing the recipe off of this should be straightforward and will take care of some the pitfalls I've mentioned and most which I didn't :)

Let me know if you need any help with that

recipes/libq/all/conandata.yml Outdated Show resolved Hide resolved
recipes/libq/all/conanfile.py Outdated Show resolved Hide resolved
class libqConan(ConanFile):
name = "libq"
description = "A platform-independent promise library for C++, implementing asynchronous continuations."
license = "Apache license 2.0"
Copy link
Member

Choose a reason for hiding this comment

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

This should be a spdx identifier listed https://spdx.org/licenses/

Suggested change
license = "Apache license 2.0"
license = "Apache-2.0"

recipes/libq/all/conanfile.py Show resolved Hide resolved
recipes/libq/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libq/all/conanfile.py Outdated Show resolved Hide resolved
@AbrilRBS AbrilRBS self-assigned this Feb 23, 2024
@conan-center-bot

This comment has been minimized.

@metalMajor
Copy link
Contributor Author

Hi! Thanks a lot for taking the time to contribute the recipe, we really appreciate it!

I've left some comments on the minor changes, but I'd suggest looking into the CMake package template https://github.com/conan-io/conan-center-index/tree/master/docs/package_templates/cmake_package - basing the recipe off of this should be straightforward and will take care of some the pitfalls I've mentioned and most which I didn't :)

Let me know if you need any help with that

Hello, I think I managed to update it based on the template. Thank for the review!

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@AbrilRBS AbrilRBS requested a review from uilianries May 4, 2024 23:28
@metalMajor
Copy link
Contributor Author

I don't get why it says libq not found in package in the test package.

@conan-center-bot

This comment has been minimized.

@metalMajor
Copy link
Contributor Author

Thank you for the review, I applied your changes, it looks good now I think 👍
I was not logged in anymore somehow, so I did not see the buttons to apply your changes(which I found strange), therefore I just copied them, and afterwards found out that I was not logged in.

recipes/libq/all/conandata.yml Outdated Show resolved Hide resolved
recipes/libq/config.yml Outdated Show resolved Hide resolved
metalMajor and others added 2 commits November 14, 2024 16:01
Co-authored-by: Uilian Ries <uilianries@gmail.com>
uilianries
uilianries previously approved these changes Nov 14, 2024
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

@uilianries uilianries requested a review from AbrilRBS November 14, 2024 16:24
@AbrilRBS
Copy link
Member

Hi @metalMajor I made some changes to get Windows building at least as static.

Now the only remaing hurdle for this PR to solve comes from the CLA bot, #22828 (comment) says that there is at least one commit authored by a different author, who would need to also sign it (Or for the commit to be rebased with a different author)

@uilianries
Copy link
Member

The use Tom Deblauwe. Probably, a second email that was used to push this PR. That email needs to sign the CLA too.

@metalMajor
Copy link
Contributor Author

I think it is solved, I removed an email address, now I put it back.

AbrilRBS
AbrilRBS previously approved these changes Nov 15, 2024
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.

Thanks a lot for taking the time to get the PR thru the finish line, and thanks for your patience while we got it reviewed, we appreciate it :)

@AbrilRBS AbrilRBS requested a review from uilianries November 15, 2024 09:43
uilianries
uilianries previously approved these changes Nov 15, 2024
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

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 you for your PR!

@uilianries uilianries merged commit a7af27e into conan-io:master Nov 15, 2024
9 checks passed
@metalMajor
Copy link
Contributor Author

Awesome!

Ahajha pushed a commit to Ahajha/conan-center-index that referenced this pull request Nov 15, 2024
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>
Ahajha pushed a commit to Ahajha/conan-center-index that referenced this pull request Nov 15, 2024
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants