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

Logfault header only library #23126

Merged
merged 29 commits into from
Apr 17, 2024
Merged

Conversation

jgaa
Copy link
Contributor

@jgaa jgaa commented Mar 17, 2024

This is my first submission to the conan-center-index.

Specify library name and version: logfault/0.5.0

I am the author of the package Logfault. It has some users, so in order
to make it simpler to use for everyone, I'm submitting this PR to include it
in the ConanCenter.


This is my first submission to the conan-center-index.
@CLAassistant
Copy link

CLAassistant commented Mar 17, 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.

@RubenRBS RubenRBS self-assigned this Mar 18, 2024
Copy link
Member

@RubenRBS RubenRBS 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 yout contribution and taking the time to submit your library to CCI, we appreciate it :)

The recipe looks great overall, the only thing on my side besides the minor review comments, would be to remoce the comments from the template to keep the recipe more readable, thanks!

recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
jgaa and others added 6 commits March 18, 2024 18:22
Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>
Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>
Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>
Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>
@conan-center-bot

This comment has been minimized.

…in as generator to ensure its correct initialization'
@conan-center-bot

This comment has been minimized.

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.

Thank you for your PR! Please, take a look on my review.

recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
jgaa and others added 3 commits March 19, 2024 18:20
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
jgaa and others added 3 commits March 19, 2024 19:26
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

@jgaa
Copy link
Contributor Author

jgaa commented Mar 22, 2024

I can't find any examples of other projects using gtest and cmake.build() for the target package in conan-center-index.

The problem with the build is that somehow the configuring of gtest does not produce (or appear so to me - I don't have the complete build logs) one of the expected CMake targets; GTest::gtest or gtest::gtest.

When I run the build locally with testing enabled, I get a message in output: Conan: Target declared 'gtest::gtest'. In the build from Conan Center, I don't see this. Just Found GTest: /home/conan/workspace/prod-v2/bsr/4772/dadbb/p/gtestf28c74556dabb/p/lib/libgtest.a.

For now I'm keeping the test code in conanfile.py, but disabling it so the project can pass. I would be good however to find the root cause so the unit tests can be ran while building at the Conan Center.

@conan-center-bot

This comment has been minimized.

recipes/logfault/config.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@valgur valgur left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM. Only some minor change suggestions.

jgaa and others added 3 commits March 25, 2024 11:24
Co-authored-by: Martin Valgur <martin.valgur@gmail.com>
Co-authored-by: Martin Valgur <martin.valgur@gmail.com>
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@jgaa
Copy link
Contributor Author

jgaa commented Mar 28, 2024

Are there any pending tasks from my side?

@jgaa
Copy link
Contributor Author

jgaa commented Apr 1, 2024

Ping @RubenRBS
Are you guys waiting for anything more from me?

@RubenRBS
Copy link
Member

RubenRBS commented Apr 1, 2024

Hi @jgaa thanks for the ping

This PR is just waiting for a review from us again - we were out on easter holidays this last week so we didn't get to do much reviewing, I'll try to get this done today :)

Copy link
Member

@RubenRBS RubenRBS left a comment

Choose a reason for hiding this comment

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

Looks good, minor question about the generate() method :)

recipes/logfault/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 29 (c43dcd21221b4a223153f7fb2dc54d841c0f09d7):

  • logfault/0.5.2:
    All packages built successfully! (All logs)

@jgaa
Copy link
Contributor Author

jgaa commented Apr 12, 2024

Ping @RubenRBS
Are you guys waiting for any changes from me?

@RubenRBS
Copy link
Member

Hi @jgaa sorry for the radio silence. There's nothing else needed from you right now, the CI systems will pick the PR up once they leave maintenance mode, thanks!

@conan-center-bot conan-center-bot merged commit 966ae31 into conan-io:master Apr 17, 2024
13 checks passed
franramirez688 pushed a commit to toge/conan-center-index that referenced this pull request Apr 23, 2024
* First take on a conan-recipe for logfault

This is my first submission to the conan-center-index.

* Corrected yaml indention

* Changed requirements. self.test_requires() failed with object has no attribute 'test_requires'

* Update recipes/logfault/all/conanfile.py

Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>

* Update recipes/logfault/all/conanfile.py

Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>

* Update recipes/logfault/all/conanfile.py

Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>

* Update recipes/logfault/all/conanfile.py

Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>

* Removed comments and 'def export_sources(self):' as suggested in code review.

* trying to fix build error: '... Check that you are using CMakeToolchain as generator to ensure its correct initialization'

* Update recipes/logfault/all/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* Update recipes/logfault/all/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* Update recipes/logfault/all/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* Update recipes/logfault/all/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* Update recipes/logfault/all/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* Changed cmake.test() to cmake.ctest(). There is no 'test' target.

* cmake.ctest() worked when I tested it locally, but failed v2_linter. Using cmake.test() with 'conan build .' with the original code checked out works. There is a 'test' target. Testing this receipie locally with 'conan create all/conanfile.py --version 0.5.0' fails because of a missing 'test' target. Changing the target of cmake.test() to 'all' works locally, and the test is executed. So trying that.

* Fixed problems with GTest link target in logfault code. Now pointing to version 0.5.1

Set tools.build:skip_test setting consistently to False. It was mixed, causing problems with test target.

* Updating version to 0.5.1

* Trying to restore the build by modifying the Cmake files in the origibnal code (bumpped to 0.5.2 to get a new tarball from github). Everything works fine locally and while building the original code with Github Actions on all platforms. But so it did for 0.5.1 as well.

* Disabling unit tests.

* Removed unit tests option

* Update recipes/logfault/all/test_package/CMakeLists.txt

Co-authored-by: Martin Valgur <martin.valgur@gmail.com>

* Update recipes/logfault/config.yml

Co-authored-by: Martin Valgur <martin.valgur@gmail.com>

* Removed 'generate' method.

---------

Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Martin Valgur <martin.valgur@gmail.com>
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.

None yet

7 participants