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 so3 package #5178

Merged
merged 10 commits into from
May 17, 2021
Merged

Add so3 package #5178

merged 10 commits into from
May 17, 2021

Conversation

mdavezac
Copy link
Contributor

@mdavezac mdavezac commented Apr 10, 2021

Closes #5154


  • 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.

recipes/so3/all/test_package/conanfile.py Outdated Show resolved Hide resolved
recipes/so3/all/conanfile.py Outdated Show resolved Hide resolved
recipes/so3/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot

This comment has been minimized.

@prince-chrismc
Copy link
Contributor

You'll need to retrigger CI, close the pr wait 10s and then re-open it 🔁

@mdavezac mdavezac closed this Apr 13, 2021
@mdavezac mdavezac reopened this Apr 13, 2021
@conan-center-bot

This comment has been minimized.

prince-chrismc
prince-chrismc previously approved these changes Apr 13, 2021
@uilianries
Copy link
Member

@mdavezac could we move this PR to Ready for Review?

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

This comment has been minimized.

Because 1.3.3 published prematurely to pipy and that's irrevocable.
@conan-center-bot

This comment has been minimized.

@mdavezac
Copy link
Contributor Author

Shouldn't we bump upstream from RC to stable before merging this PR?

Yes, that's the plan. It's in rc because there's a github action to push full releases to pypi

@conan-center-bot

This comment has been minimized.

Co-authored-by: theirix <theirix@gmail.com>
@conan-center-bot

This comment has been minimized.



class So3Conan(ConanFile):
name = "so3"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name = "so3"
name = "astro-informatics-so3"

I would rename it. The project SO3 provided by Astro Informatics is not so popular, only 3 github stars and there are other similar projects with same name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. We'll go with so3-transform, if that's allright. It will have to wait a few days though.

Copy link
Contributor

Choose a reason for hiding this comment

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

@uilianries unfortunately, the name so3 might be too generic (e.g. it stands for sulfur trioxide and few other things)

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/conan-io/conan-center-index/blob/master/docs/faqs.md#what-is-the-policy-on-recipe-name-collisions

https://github.com/conan-io/conan-center-index/blob/master/docs/faqs.md#should-reference-names-use---or-_

For libraries with a too generic name, like variant, the name of the organization can be used as prefix separated by a -, like mpark-variant

Our "rule" is the author/organization if you dont mind following the suggestion above

@conan-center-bot

This comment has been minimized.

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.

After the name change (I agree so3 is too generic) I would say the recipe is ready to go 🚀

@conan-center-bot

This comment has been minimized.

prince-chrismc
prince-chrismc previously approved these changes May 14, 2021
czoido
czoido previously approved these changes May 14, 2021
@mdavezac mdavezac marked this pull request as draft May 14, 2021 14:22
@mdavezac
Copy link
Contributor Author

Converting to draft until I've released the upstream and updated conandata.yml.

@mdavezac mdavezac dismissed stale reviews from czoido and prince-chrismc via 7f5c0a7 May 14, 2021 14:55
@mdavezac mdavezac marked this pull request as ready for review May 14, 2021 14:55
@mdavezac
Copy link
Contributor Author

Okay, upstream has been released and this recipe now points to it.

@conan-center-bot
Copy link
Collaborator

All green in build 13 (7f5c0a7dc41bf8dfe9fd27b6ad962aaa55ff9df9):

  • astro-informatics-so3/1.3.4@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit f6f4fbe into conan-io:master May 17, 2021
@mdavezac mdavezac mentioned this pull request May 17, 2021
@mdavezac mdavezac deleted the issue/5154 branch May 18, 2021 08:40
@mdavezac
Copy link
Contributor Author

Thanks for all the help and comments!

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.

[request] S03/1.3.3
8 participants