-
Notifications
You must be signed in to change notification settings - Fork 14
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
adding release_and_publish_pypi.yaml file, may need editing but unsur… #167
adding release_and_publish_pypi.yaml file, may need editing but unsur… #167
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for starting this! I added some comments
- name: Build package | ||
run: | | ||
python -m build | ||
python setup.py bdist_wheel sdist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this second build step might be redundant, python -m build
should build both the wheels and the source distributions that we need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed redundant call
python setup.py bdist_wheel sdist | ||
- name: Upload to twine test | ||
run: | | ||
twine upload -r testpypi dist/* --verbose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll also want to use a secret here with the same __token__
username and a new secrets.TEST_PYPI_API_TOKEN
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, fixed this
with: | ||
user: __token__ | ||
password: ${{ secrets.PYPI_API_TOKEN }} | ||
- name: Add a git tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can come out of this workflow since it's triggered by a release and so we'll already have built a tag for this release when this gets called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed step to add a git tag, pushing changes up
…uild step, added test secret token to test twine publish, lastly removed git tag
Great, these changes look good, thanks for implementing them. I think it could make sense to test this towards the end of the day when we have some more changes incorporated, releasing a new patch version. |
Signed-off-by: Nicholas Reinicke <[email protected]>
…e at this time
Took a first crack at creating this but this is my first attempt at a github action so I am anticipating this will need some additional commits