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

DOC: plugin package setup add PEP 621 example #5626

Merged
merged 5 commits into from
Sep 5, 2022

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Sep 5, 2022

Test and update section redirect from aiida-tutorial-intro.
Change entry points set from setup.py to pyproject.toml as PEP 621

Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

thanks for the update @unkcpz , just one comment

@@ -332,6 +333,37 @@ With your ``calculations.py`` and ``parsers.py`` files at hand, let's register e
Strictly speaking, ``aiida-diff-tutorial`` is the name of the *distribution*, while ``aiida_diff_tutorial`` is the name of the *package*.
The aiida-core documentation uses the term *package* a bit more loosely.

* Or using ``pyproject.toml`` for new package:
Copy link
Member

@ltalirz ltalirz Sep 5, 2022

Choose a reason for hiding this comment

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

For a howto I would suggest to stick with one option.

The pyproject.toml is a little more verbose and I wonder whether we need to specify flit as the build backend even in this minimalistic example...
but if the consensus is that the pyproject.toml is the way to go, you can remove the setup.py.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the quick reply. I'd vote for only keep pyproject.toml since the cookie-cutter is already only has pyproject.toml and the 'plugin packaging section is changed to only conform with PEP 621 in this PR.
Most of cases, beginner users would simply copy and paste the code to have a taste, and then for creating a new plugin, the cookie-cutter should be always the starting point.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's keep only the pyproject.toml.

As mentioned you can experiment with removing the flit stuff to keep things a bit more "minimal" but that's up to you.

@unkcpz unkcpz requested a review from ltalirz September 5, 2022 10:22
@ltalirz ltalirz self-requested a review September 5, 2022 10:55
Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

thanks, changed the wording a little bit; good to go now

@unkcpz
Copy link
Member Author

unkcpz commented Sep 5, 2022

Thanks a lot @ltalirz

Seems some CI tests are not running, how should I trigger them?

@ltalirz
Copy link
Member

ltalirz commented Sep 5, 2022

I think the relevant tests were run (only docs changed)

image

and it's safe to squash & merge now.
I'm travelling today - if you are not allowed to do this and need help, please ping another maintainer (you should get these right though in my view if you don't have them already)

@unkcpz
Copy link
Member Author

unkcpz commented Sep 5, 2022

if you are not allowed to do this and need help, please ping another maintainer (you should get these right though in my view if you don't have them already)

Okay, I think I don't have the permission to merge without all required CI tests are passed. @ramirezfranciscof Can you check this? thanks!

@ramirezfranciscof
Copy link
Member

ramirezfranciscof commented Sep 5, 2022

@unkcpz I just made you admin, check if now you can merge it

(Edit: note that now the branch is also out of date so you need to take care of that beforehand)

@unkcpz unkcpz merged commit b38f428 into aiidateam:main Sep 5, 2022
@unkcpz unkcpz deleted the doc/plugin-codes branch September 5, 2022 19:51
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.

3 participants