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.

Sign up for GitHub

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

Jump to bottom

Add a guide about publishing dists via GH Actions #647

Merged
merged 17 commits into from
Sep 27, 2019
Merged

Add a guide about publishing dists via GH Actions #647

merged 17 commits into from
Sep 27, 2019

Conversation

webknjaz
Copy link
Member

@webknjaz
Copy link
Member Author

@pradyunsg @cjerdonek ^

hugovk
hugovk reviewed Sep 16, 2019
View reviewed changes
Copy link
Contributor

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Looks good, some minor suggestions.

source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
@webknjaz
Copy link
Member Author

Folks, this is ready for a new round of reviews. I've factored out the workflow sample into a separate file and addressed all the major concerns.

hugovk
hugovk reviewed Sep 18, 2019
View reviewed changes
Copy link
Contributor

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Looks good, just a few more minor bits!

source/guides/github-actions-ci-cd-sample/publish-to-test-pypi.yml
.
# Actualy publish to PyPIs
- name: Publish 📦 to Test PyPI
uses: pypa/gh-action-pypi-publish@master
Copy link
Contributor

Choose a reason for hiding this comment

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

Once there's a release, include the version number here.

We strongly recommend that you include the version of the action you are using by specifying a Git ref, SHA, or Docker tag number. If you don't specify a version, it could break your workflows or cause unexpected behavior when the action owner publishes an update.

  • Using the commit SHA of a released action version is the safest for stability and security.
  • Using the specific major action version allows you to receive critical fixes and security patches while still maintaining compatibility. It also assures that your workflow should still work.
  • Using the master branch of an action may be convenient, but if someone releases a new major version with a breaking change, your workflow could break.

https://help.github.com/en/articles/workflow-syntax-for-github-actions#jobsjob_idstepsuses

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I'd also mention it somewhere in the README of the action itself.

source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
@webknjaz
Copy link
Member Author

@di @ncoghlan I think this is waiting for merge now. Mind taking a look? TIA.

@webknjaz @hugovk
Apply style and grammar corrections
5fa47af 
Co-Authored-By: Hugo van Kemenade <hugovk@users.noreply.github.com>
di
di reviewed Sep 19, 2019
View reviewed changes
Copy link
Member

@di di left a comment

Choose a reason for hiding this comment

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

This looks pretty good.

I'm not 100% sure that we should advise users to publish to TestPyPI on every push, but I think it's probably OK.

source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/github-actions-ci-cd-sample/publish-to-test-pypi.yml Outdated Show resolved Hide resolved
source/guides/github-actions-ci-cd-sample/publish-to-test-pypi.yml Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
source/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows.rst Outdated Show resolved Hide resolved
@webknjaz webknjaz mentioned this pull request Sep 19, 2019
Merged
webknjaz added a commit to pypa/gh-action-pypi-publish that referenced this pull request Sep 19, 2019
@webknjaz
Test PyPI -> TestPyPI in README
9cebe9a 
PR #13 by @hugovk

Ref: pypa/packaging.python.org#647 (comment)
webknjaz and others added 3 commits September 19, 2019 12:38
@webknjaz @di
Fix TestPyPI spelling
9a15647 
Co-Authored-By: Dustin Ingram <di@users.noreply.github.com>
@webknjaz @di
Use long versions of words in the guide
bdcc968 
Co-Authored-By: Dustin Ingram <di@users.noreply.github.com>
@webknjaz @di
Apply style fixes from code review
4b1e6d2 
Co-Authored-By: Dustin Ingram <di@users.noreply.github.com>
@webknjaz webknjaz requested a review from di September 19, 2019 13:50
@webknjaz webknjaz mentioned this pull request Sep 19, 2019
Closed
10 tasks
@webknjaz
Copy link
Member Author

@di would you please take a look?

pradyunsg
pradyunsg approved these changes Sep 26, 2019
View reviewed changes
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

I think this is good-enough to publish now; we can always iterate and improve this as we go.

@webknjaz webknjaz mentioned this pull request Sep 26, 2019
Closed
@breznak breznak mentioned this pull request Sep 26, 2019
Closed
4 tasks
ncoghlan
ncoghlan approved these changes Sep 27, 2019
View reviewed changes
Copy link
Member

@ncoghlan ncoghlan left a comment

Choose a reason for hiding this comment

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

Concur with @pradyunsg here - I think there are some interesting questions on specifics in the remaining comments, but I also think they can be tackled via a follow-up PR.

@ncoghlan ncoghlan merged commit b852020 into pypa:master Sep 27, 2019
@webknjaz
Copy link
Member Author

Thanks, Nick! I'm also aware of other problems that need to be addressed and I'm going to solve those too, in the follow-up PRs.

@pfmoore pfmoore mentioned this pull request Sep 9, 2020
Closed
@hugovk

This comment was marked as outdated.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugovk hugovk hugovk approved these changes

@pradyunsg pradyunsg pradyunsg approved these changes

@ncoghlan ncoghlan ncoghlan approved these changes

@pfmoore pfmoore Awaiting requested review from pfmoore

@di di Awaiting requested review from di

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a usage guide to packaging.python.org
6 participants
@webknjaz @hugovk @di @ncoghlan @pfmoore @pradyunsg