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

added pypi-ci #36

Merged
merged 5 commits into from
Nov 13, 2023
Merged

Conversation

CarlottaSartore
Copy link
Contributor

This should address #35

Now on releases, urdf-modifier should be release on pypi

@CarlottaSartore
Copy link
Contributor Author

CarlottaSartore commented Nov 9, 2023

C.C. @Nicogene @traversaro @mfussi66

@traversaro
Copy link
Member

I guess we need to add a token secret here. I asked back in time to PyPI to get an organization, but I need to check what happen to that request. In the meanwhile I guess we need to add a personal token. @Nicogene @mfussi66 if you want I can handle that, let me know if that is ok.

@Nicogene
Copy link
Member

Nicogene commented Nov 9, 2023

I guess we need to add a personal token. @Nicogene @mfussi66 if you want I can handle that, let me know if that is ok.

Yes sure!

@mfussi66
Copy link
Member

I guess we need to add a token secret here. I asked back in time to PyPI to get an organization, but I need to check what happen to that request. In the meanwhile I guess we need to add a personal token. @Nicogene @mfussi66 if you want I can handle that, let me know if that is ok.

No problem!

@traversaro
Copy link
Member

I added the tokens, the PR is ready for review. @Nicogene @CarlottaSartore @mfussi66 please give me your pypi handles to be added to the project, thanks!

@Nicogene
Copy link
Member

I added the tokens, the PR is ready for review. @Nicogene @CarlottaSartore @mfussi66 please give me your pypi handles to be added to the project, thanks!

Mine is Nicogene

@traversaro
Copy link
Member

Nicogene

Ok, I will add you as soon as the first release exists, otherwise before there is no pypi project to add you to.

README.md Outdated

```bash

pip install "urdfModifiers @ git+https://github.com/icub-tech-iit/urdf-modifiers"
pip install urdfModifiers
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
pip install urdfModifiers
pip install urdf-modifiers

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Argh, but the python module name is urdfModifiers .

Copy link
Member

Choose a reason for hiding this comment

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

To clarify why I do not like urdfModifiers, camel case is not suggested in PEP8 guidelines, see https://peps.python.org/pep-0008/#package-and-module-names, and not so common at all (see https://pypistats.org/top). Anyhow, I don't want to create more friction than necessary, so if you want to go forward with urdfModifiers, feel free to proceed.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking more about this, I think we should at least call the package name urdf-modifiers . We can always change the module name later from urdfModifiers to urdf-modifiers in a backward compatible way, but changing python package name would be complex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 71f49ee and 8747f15

@mfussi66
Copy link
Member

I added the tokens, the PR is ready for review. @Nicogene @CarlottaSartore @mfussi66 please give me your pypi handles to be added to the project, thanks!

Mine is mfuzz.

@CarlottaSartore
Copy link
Contributor Author

I added the tokens, the PR is ready for review. @Nicogene @CarlottaSartore @mfussi66 please give me your pypi handles to be added to the project, thanks!

Mine is CarlottaSartore

Comment on lines 4 to 5
author = "TODO"
author_email = TODO
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
author = "TODO"
author_email = TODO
author = "Germán Rodriguez, Alexandre Antunes"
author_email = "[email protected]"
mantainer = "Nicolo Genesio, Mattia Fussi"
mantainer_email = "[email protected], [email protected]"

Copy link
Member

Choose a reason for hiding this comment

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

cc @Nicogene @mfussi66 please double check, thanks!

fyi @CarlottaSartore

Copy link
Member

Choose a reason for hiding this comment

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

Ok for mine, I am not sure about the German email :D

Copy link
Member

Choose a reason for hiding this comment

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

Ok for mine, I am not sure about the German email :D

I got it from LinkedIn.

setup.cfg Outdated Show resolved Hide resolved
Co-authored-by: Silvio Traversaro <[email protected]>
@Nicogene Nicogene merged commit a5c3e75 into icub-tech-iit:master Nov 13, 2023
4 checks passed
@traversaro
Copy link
Member

Thanks @Nicogene ! Can we publish a new release to check if the PyPI upload is working fine?

@Nicogene
Copy link
Member

@Nicogene
Copy link
Member

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.

4 participants