-
-
Notifications
You must be signed in to change notification settings - Fork 89
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 major-minor-release workflow #45
base: unstable/v1
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,33 @@ | |||
name: "Publish Major-Minor-Tags" | |||
on: | |||
push: |
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.
Honestly, I'm lately preferring the create
event because it has closer semantics to what we're attempting to trigger. But using it requires a post-condition.
WDYT?
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 mostly use push
due to sheer habit, there were a lot fewer triggers (at least in the docs) when I started adopting gh-actions as soon as they came out of beta.
The docs are kinda unclear on how to use it, would simply changing push
for create
work? Or what is the proper pattern, only to trigger on tags that start with v
I guess semantically the closes would be to use release
.
on:
release:
types: [published]
I use this trigger for my actions and it works fine, but I'm not sure how this will play out with dev
, pre
and post
tags, since those will most likely not published.
The other release
type to use would be created
, which wouldn't force one to release on the markedplace, but it could still happen by accident (I think to remember that the checkbox is checked by default after fist publishing).
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.
The usage of create
is
It's
on:
create:
jobs:
job-name:
if: github.event.ref_type == 'tag' # to filter out branch creation
(because it doesn't have "native" filters)
I usually create the tags using local Git and push them from CLI. And only after that I publish releases. So I'm not sure if release
would be suitable. OTOH this workflow could auto-publish a release on tag creation. You already have a way of saying if the "pre-release" checkbox should be set.
Also, I'm fine with publishing pre-releases to Marketplace. I've done it before.
See https://github.com/pypa/gh-action-pypi-publish/releases/tag/v1.0.0a0 — it has a pre-release checkbox. And it's marketplace page https://github.com/marketplace/actions/pypi-publish?version=v1.0.0a0 also has a pre-release marker, just like PyPI.
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 auto-publishing is a bit too much automation since the release messages often need some handcrafted work. e.g. markdown formatting, which can't be extracted from the tag.
As for the published
event, I don't know how github determines what the current release is, my guess is that it sorts them by published date. If that assumption is correct a post
release could mess up what version is displayed on the marketplace. E.g. you have a v2.0.0
release and after that release v1.4.1.post0
to trigger the workflow, v1.4.1.post0
might be shown a current version. So IMHO [created, edited]
would be closer to the desired behavior.
Also using release
would make it more reasonable to add a link to the release to the created tags (see #45 (comment)).
As for the semantics of create
vs. push
, I think this boils down to different reference systems remote
vs. locally
(sorry physics student here 😆 ).
I personally prefer the more concise syntax of push
, where
on:
push:
tags:
- "v*"
gives you the same functionality as
on:
create:
jobs:
job-name:
if: github.event.ref_type == 'tag' && startsWith( github.ref, 'v')
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'm not sure about the markdown part. First of all, it's possible to have tag annotations with markdown baked-in but I wouldn't care about it too much: when you mark a release as published to the Marketplace, the new version appears there but the release description does not show up there. Instead, it just contains README content.
So it's okay to just auto-publish and then, if needed, I'd just correct it.
FWIW we shouldn't care about ordering the tags: it'll be like this regardless of whether a robot or a human makes the release... And we're not going to avoid putting .post
tags on the marketplace while moving the major tag there in Git.
54cc114
to
5baaafc
Compare
git config user.email 'github-actions[bot]@users.noreply.github.com' | ||
git config user.name 'github-actions[bot]' | ||
git tag --annotate '${{ env.major_version }}' \ | ||
--message='original tag: ${{ env.original_tag_name }}' |
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.
have you considered appending the message from the original tag too?
--message='original tag: ${{ env.original_tag_name }}' | |
--message='original tag: ${{ env.original_tag_name }}' \ | |
--message="$(git tag -l --format='%(contents)' '${{ env.original_tag_name }}')" |
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.
This would only work as intended if the tag had an additional message to it or?
E.g. v1.4.0
works fine
$ git tag -l --format='%(contents)' 'v1.4.0'
Merge PR #39
This change exposes an extra GitHub Action input called `verbose`. It
allows to see `twine upload` detailed output if needed for debugging.
while v1.4.1
doesn't
$ git tag -l --format='%(contents)' 'v1.4.1'
v1.4.1
-----BEGIN PGP SIGNATURE-----
.
.
.
-----END PGP SIGNATURE-----
IMHO adding a link to the release would be more informative.
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.
In the second case, v1.4.1
is also an additional message because all signed tags are also annotated. While I understand the sentiment of having a link, I'm not sure if it could be achieved easily. Another thing that would be useful is the commit hash where the tag points at (security-wise).
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.
With some conditions, creating a link would be pretty straight forward:
- release name always needs to be the same as tag name
- workflow triggers on
on: release: types: [created, edited]
If those conditions are met, the link would be
https://github.com/pypa/gh-action-pypi-publish/releases/tag/${{ env.original_tag_name }}
As for adding the SHA1 of the commit, this only provides additional information in a CLI context, since the github website already points to the underlying commit (see test tag v2.0.0).
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.
As for adding the SHA1 of the commit, this only provides additional information in a CLI context, since the github website already points to the underlying commit (see test tag v2.0.0).
Besides being a pointer, it's also proof that it was created by the workflow and not manually, overriding it to some malicious commit.
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
8d6be06
to
54e3ca6
Compare
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
534a0e6
to
22ec670
Compare
22ec670
to
aabe83c
Compare
@s-weigand it has recently been brought to my attention that rewriting tags is a terrible idea. They are supposed to be immutable. It's branches that actually need to be used as a floating pointer. It seems like the best way of implementing this is to actually use branches with names like |
@webknjaz While it is true that the idea behind tags is that they are immutable, github themselves rewrites them for major releases of their own actions (e.g. https://github.com/actions/cache/releases). Also, the tag of the exact release stays immutable, so the way I see minor or major tags, in general, isn't as strict tag but more like a convenience thing equivalent to something |
Well, I'd not idealize what GH does — they often make questionable decisions. I think it's better to have branches called And yes, I agree that patch versions should be used mostly — dependabot even knows how to bump those. But what I'm saying is that all the tags must stay immutable over time, no exceptions. Just because somebody else breaks the rules doesn't mean that it's a good idea to subvert Git's ideology. They still haven't solved the security issue that arises from using moving targets on the platform level meaning that all the users have to either use trusted Actions or use commit SHAs and audit every change on their own since there's no fair guarantee that some random action author won't go rogue and retargets a tag or a branch to some malicious commit (it's even more dangerous when done silently and temporarily — it leaves almost no trace in logs). |
Ok point taken. Anyway, WDYT about having the branches all in a folder e.g. |
Yeah, that's a good idea. Except I'd maybe call those
Fair enough. |
- name: Set up Python 3.8 | ||
uses: actions/setup-python@v2 | ||
with: | ||
python-version: 3.8 |
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 guess it's time to bump this
- name: Set up Python 3.8 | |
uses: actions/setup-python@v2 | |
with: | |
python-version: 3.8 | |
- name: Set up Python 3.9 | |
uses: actions/setup-python@v2 | |
with: | |
python-version: 3.9 |
@s-weigand so I was thinking it may be a good idea to have branches like |
Here is the promised workflow, for major and minor tag releases, which works with PEP440 versions.
Creates major+minor tags on
v2.0.0
)v2.0.0.post0
)Does not creates major+minor tags on
v2.0.0.dev0
)v2.0.0.rc0
)Also, it saves the original tag name as an annotation in the major and minor tags.
I tested some example tags on my fork.