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

Add s2let/2.2.4 package #5550

Merged
merged 4 commits into from
Jun 14, 2021
Merged

Conversation

mdavezac
Copy link
Contributor

Specify library name and version: s2let/2.2.4

s2let is a fast wavelet transform on the sphere. It can be used for radio-inteferometry, VR, or anywhere a signal on the sphere is analyzed. This PR is a follow up to ssht and astro-informatics-so3 added recently.


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot

This comment has been minimized.

@SSE4 SSE4 requested a review from uilianries May 18, 2021 14:33
recipes/s2let/all/conandata.yml Outdated Show resolved Hide resolved
recipes/s2let/all/conanfile.py Outdated Show resolved Hide resolved
recipes/s2let/all/conanfile.py Show resolved Hide resolved
recipes/s2let/all/conanfile.py Outdated Show resolved Hide resolved
recipes/s2let/all/conanfile.py Show resolved Hide resolved
recipes/s2let/all/conanfile.py Outdated Show resolved Hide resolved
recipes/s2let/all/conanfile.py Outdated Show resolved Hide resolved
recipes/s2let/all/conanfile.py Show resolved Hide resolved
recipes/s2let/all/conanfile.py Outdated Show resolved Hide resolved
recipes/s2let/all/conanfile.py Show resolved Hide resolved
recipes/s2let/all/test_package/CMakeLists.txt Outdated Show resolved Hide resolved
recipes/s2let/all/test_package/CMakeLists.txt Outdated Show resolved Hide resolved
@conan-center-bot

This comment has been minimized.

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
@mdavezac
Copy link
Contributor Author

mdavezac commented Jun 9, 2021

Thanks @SpaceIm, I've been meaning to puzzle out how to make cfitsio optional but couldn't quite figure it out!

@conan-center-bot

This comment has been minimized.

SpaceIm
SpaceIm previously approved these changes Jun 9, 2021
Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

Changes requested:

  • do not build tests
  • use final release instead of RC

recipes/s2let/all/conanfile.py Show resolved Hide resolved
recipes/s2let/all/conandata.yml Show resolved Hide resolved
@prince-chrismc
Copy link
Contributor

In his defense, if you look at the last PRs https://github.com/conan-io/conan-center-index/pulls?q=is%3Apr+is%3Aclosed+author%3Amdavezac
The changes between the release have been improving things from a OSS distribution perspective.

We have seen code not compiling but most of it is we provided improvements and the patches get integrated before the PR is merged.
You can see that here with the commit messages astro-informatics/so3@1.3.4rc1...v1.3.4

If project maintainers work with us to incorporate patches, I think it's better then the messiness of #3951

I feel OP has respected the rules but it's always good to have a refresher.

@jgsogo
Copy link
Contributor

jgsogo commented Jun 11, 2021

I didn't mean to be rude, but from time to time I need to play policeman here. We all agree that this is not a CI service, and we all agree that the way this PR and related recipes are being added to CCI is not affecting badly other PRs that are being built (time/resources)... but, also, we all need to understand that this scenario is something that could happen and we need to pay attention to it.

It is great to see that the pull-requests to ConanCenter and the reviews are contributing to improving the original project. It is, without any doubt, one of the best things that could happen here. And I truly believe that we all are contributing to improving the C++ ecosystem, and I'm so grateful for it.

But (there is always a but), I need to remember some rules from time to time, not based on my own feelings, neither on how big/small the project is. We are not closing the pull-request or banning anyone here, so please, I apologize if it sounded rude. Please, keep pushing changes until it is ready to merge 🚀

@mdavezac
Copy link
Contributor Author

@jgsogo, thanks for your review and the policy remainder. Just to confirm what @prince-chrismc said, the upstream has been in rc while waiting for any necessary changes arising from this PR and only this PR. The upstream has it's own CI with more tests than are suitable for conan-center. So the intent is not to check for anything here except whether we can build with conan.

@conan-center-bot
Copy link
Collaborator

All green in build 4 (d56263b803ab2f0f60fe3adb36b81cc55ceb7388):

  • s2let/2.2.3@:
    All packages built successfully! (All logs)

Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

astro-informatics!! 🚀 🚀 🚀

@conan-center-bot conan-center-bot merged commit 115e061 into conan-io:master Jun 14, 2021
@mdavezac mdavezac deleted the feature/5549 branch June 14, 2021 10:59
@mdavezac
Copy link
Contributor Author

Thank you for all your comments and reviews!

madebr pushed a commit to madebr/conan-center-index that referenced this pull request Jun 21, 2021
* add s2let/2.2.3

* Apply changes from code review comments

* Apply suggestions from code review

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

* Update URL and SHA for upstream release

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
@mdavezac mdavezac mentioned this pull request Sep 27, 2021
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.

7 participants