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

Add major version at release #22

Open
s-weigand opened this issue Dec 19, 2019 · 19 comments
Open

Add major version at release #22

s-weigand opened this issue Dec 19, 2019 · 19 comments
Assignees
Labels
enhancement New feature or request

Comments

@s-weigand
Copy link

To make it easier for users to adapt to breaking changes in the action, having major version tags to rely on, instead of master, is a good thing for users and developers.
I recently discovered this awesome action which does exactly this, fully automated on relese if you set you workflow up like this.
Just wanted to share that info and action, to save you the research.

P.S.: Thanks for this action, it works like a charm 😄

@webknjaz webknjaz self-assigned this Jan 20, 2020
@webknjaz webknjaz added the enhancement New feature or request label Jan 20, 2020
@webknjaz
Copy link
Member

webknjaz commented Apr 2, 2020

fully automated on relese if you set you workflow up like this .

This sounds nice but is a security hazard (explained here why #27 (comment)). I'm thinking about how to solve this best but don't have a timeline for completing such setup.

@s-weigand
Copy link
Author

I see your point and thanks for sharing the post.
But on the other hand, IMHO you have to trust some organizations like github or pypa, that the tools they publish aren't malicious. Actions from pypa aren't just 'random @github actions found on the markedplace' as mentioned in the post.
In theory you would have the same security problem when updating twine.

To minimize the risk that a 3rd party action injects malicious code into your release, you could just fork release-github-actions. This way you have the control over what is done on release.

It might still be worth mentioning the security risks and encourage users to use the commit sha, since as the post stated patch version tags are as hazardous and major versions.

@hugovk
Copy link
Contributor

hugovk commented Oct 7, 2020

A v1 tag is technically insecure, but so is v1.4.1.

A lack of v1 means people use master, arguably less secure than a tag.

Of course people should use a SHA or fork but they don't...

(The pro tip on the README recommends SHA or tags over master. It should recommend SHA/fork over tags and master.)

@webknjaz
Copy link
Member

webknjaz commented Oct 7, 2020

@hugovk fair enough! I guess I should find time to think through the options and finally solve this request once and for all...

@pradyunsg
Copy link
Member

@webknjaz Have you thought through this?

I think it'd be good to have a v1 tag, following the model that actions/ repositories (as well as basically every other popular action that I know of) follow here.

@webknjaz
Copy link
Member

webknjaz commented Nov 1, 2020

@pradyunsg you are right. I think I want it to be introduced along with the detailed explanation of the security considerations. Not sure if I'll be up for it right now — I need to recover from the today's drama. But feel free to remind me in a day or two...

@s-weigand
Copy link
Author

@webknjaz If you want I can make a PR with a workflow to generate/automate the major version tags and you can add a detailed explanation of the security considerations. Sharded works means less work for all.
Also, good recovery 😄

@pradyunsg
Copy link
Member

pradyunsg commented Nov 1, 2020

@s-weigand Please go ahead. I can't promise that @webknjaz would use it as is, but I'm certain that it'd be helpful to have it, even if it only serves as a starting platform! ^>^

@webknjaz
Copy link
Member

webknjaz commented Nov 1, 2020

Thanks, I'm honestly not sure what workflow I want but if you present me with options, I'll appreciate that. I recall that I looked at some automation in the past and didn't like something about it, can't remember what exactly, though.

@webknjaz
Copy link
Member

webknjaz commented Nov 1, 2020

Oh, I remember: haya14busa/action-update-semver that was submitted to another action I have doesn't support PEP440 versions, like .dev0 or .post0 suffixes that I like to use. I still feel that it's important to support it.

@s-weigand
Copy link
Author

Oh, I remember: haya14busa/action-update-semver that was submitted to another action I have doesn't support PEP440 versions, like .dev0 or .post0 suffixes that I like to use. I still feel that it's important to support it.

Good to know, I will keep that in mind 😄
I guess .dev shouldn't trigger a major tag, while .post should.

@webknjaz
Copy link
Member

webknjaz commented Nov 1, 2020

Yep. I think JS folks who write these actions don't really know about anything other than semver. I think it should be fairly easy to come up with a workflow that is triggered on tag create events, installs packaging, analyzes the tag with Python and force-pushes a major version tag or bails.

@s-weigand
Copy link
Author

Since this is a docker action and we save all the js bundeling stuff, the main points are parsing the version (e.g. using pkg_resources.parse_version and passing the tag as a CLI arg) and force pushing the tags if needed.
I will give it a go tomorrow or so. 😄

@webknjaz
Copy link
Member

webknjaz commented Nov 1, 2020

But the pipeline for this specific action does not run it. We're talking about a separate pipeline that would just operate the GitHub repo, not the Action inside of it, right?

@s-weigand
Copy link
Author

What I was talking about was just a python script, which decides whether to push minor/major tags, and does it if it should.

@pradyunsg
Copy link
Member

pkg_resources.parse_version

Please use packaging.version instead -- https://packaging.pypa.io/en/latest/version/.

@webknjaz
Copy link
Member

webknjaz commented Nov 1, 2020

Yep! That's what I meant when I said install packaging earlier.

@s-weigand since it's supposed to be used within GHA only, I'd just embed that into YAML with shell: python YOLO style instead of adding an extra file :D

@s-weigand
Copy link
Author

Sadly YOLO style doesn't work since shell: python is python 2.7 (RIP) and I want my f-strings 🤣

@webknjaz
Copy link
Member

webknjaz commented Nov 3, 2020

@s-weigand that is not entirely correct. shell: python is whatever points to python currently. If you setup-python before that, it can be 3.9 too. Example: https://github.com/webknjaz/intersphinx-untangled/blob/c9223eb/.github/workflows/build-gh-pages.yml#L16-L30. Also: https://github.com/sphinx-contrib/sphinxcontrib-towncrier/blob/413f87d/.github/workflows/tox-tests.yaml#L54 (yes, that is an f-string!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants
@webknjaz @hugovk @pradyunsg @s-weigand and others