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 an automated release workflow using Github Actions #331

Merged
merged 4 commits into from
May 6, 2024

Conversation

AdamWill
Copy link
Member

@AdamWill AdamWill commented May 4, 2024

This adds a release workflow, and updates tool config a bit to make the latest incarnation of bumpversion happy. I've tested that you can do bump-my-version bump major on top of this and it looks like it does everything we'd want - update all the appropriate version strings and create a tagged commit. So I hope that, once we merge this, we can submit another PR created by editing CHANGELOG.md then running bump-my-version bump major, then merge that PR and the release should happen magically.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
@AdamWill AdamWill mentioned this pull request May 4, 2024
@coveralls
Copy link
Collaborator

coveralls commented May 4, 2024

Coverage Status

coverage: 70.065% (-4.7%) from 74.805%
when pulling a78a6f8 on AdamWill:release-workflow
into a742974 on mwclient:master.

@AdamWill
Copy link
Member Author

AdamWill commented May 4, 2024

BTW, I got notified of a failed run of the new action for this PR. Looking at it, I think it failed because it ran from my fork (AdamWill/mwclient), not from the project (mwclient/mwclient). Of course I configured the pypi project to only allow mwclient/mwclient to publish, not forks :) I'm not sure if this would happen every time anyone pushed a commit to their fork, if we merged this; if so that would be rather annoying and we'd have to find a way to avoid it...

@waldyrious
Copy link
Member

Yeah, I think that does happen indeed — every now and then I get notifications from GitHub about actions being disabled on forks I had created months earlier to submit a PR. I'm not sure there is a solution, but it sounds annoying enough that probably people have at least investigated that possibility. Unfortunately that's the extent of my knowledge about this.

@AdamWill
Copy link
Member Author

AdamWill commented May 4, 2024

OK, so here's a tweaked version that should avoid running the actions on pushes to anything but the main repo. I really hope the syntax of the if line for publish-to-pypi is correct, unfortunately I'm not sure we can test it any way other than trying to use it for the first time.

BTW, it seems like since ~2021 github asks you if you want to enable actions on a fork when you create it, but forks created before then have actions turned on by default.

@AdamWill
Copy link
Member Author

AdamWill commented May 4, 2024

I'm also concerned the publish-to-testpypi job might fail every time it's run after the first, between releases, because it looks like it'll keep trying to publish the same version over and over...but it's in the sample config, so...hmm. I guess we can take it out if it does that...

@AdamWill
Copy link
Member Author

AdamWill commented May 4, 2024

Ah, yeah, it looks like I'm right about that. Will drop the publish-to-testpypi bit, then.

@AdamWill AdamWill force-pushed the release-workflow branch 3 times, most recently from 82f99fa to 71ffac9 Compare May 4, 2024 20:21
@AdamWill
Copy link
Member Author

AdamWill commented May 4, 2024

OK, there, I think that should be good?

@AdamWill
Copy link
Member Author

AdamWill commented May 4, 2024

well, added a comment to setup.py explaining how to do a release.

setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@waldyrious
Copy link
Member

Looks great! I've left a few minor edit suggestions as inline comments, but other than that, this should be ready to go :)

@waldyrious
Copy link
Member

Btw, what a nice coincidence that you became active precisely during the Wikimedia Hackathon, so I was available to review your changes quickly. I also happened to bump into @WolfgangFahl, who's been active in the project's issues, so overall it was a nice weekend for mwclient! 😁

It's much nicer to have the main repo as origin and your fork as
another remote you just push PR branches to. That way you don't
have to deal with resyncing the master branch of your fork all
the time.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
The latest incarnation of bumpversion is bump-my-version, which
considers .cfg configuration "deprecated" and wants it moved
to TOML, so here we go. We may as well move pytest config at
the same time as pytest has supported it for ages. We can drop
the "universal wheel" thing as that was about supporting Python
2 and 3 in the same wheel, and we don't support Python 2 any
more.

flake8 does not support pyproject.toml and it doesn't look like
you can specify aliases for setup.py there either, so we'll leave
those two in setup.cfg for now. Maybe in future we can get rid
of them.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
Copy link
Member

@waldyrious waldyrious left a comment

Choose a reason for hiding this comment

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

Looks great! Feel free to merge at your convenience.

This is based on
https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/#defining-a-workflow-job-environment
. It's the sample config from there, with versions updated and
our project name substituted in the appropriate places. I dropped
the publish-to-testpypi bit because of
pypa/packaging.python.org#804 , and left
the Github release part left out for now. We can add that later if
we like, but we never published releases to Github before, so it
doesn't seem required yet. I also tweaked the conditionals a bit
to avoid running the build job on forks and publish only tags
that start with 'v', as that's our convention for versions.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
@AdamWill
Copy link
Member Author

AdamWill commented May 6, 2024

actually, just sent one final change: made the conditional for the two remaining steps in the workflow the same, because now we don't have the publish-to-testpypi step there's really no point building a dist for every push. if you can just sign off on that too, I'll merge. thanks! @waldyrious

@waldyrious
Copy link
Member

That looks good, and makes sense. But just to be sure, did you see the inline comment above?

@AdamWill
Copy link
Member Author

AdamWill commented May 6, 2024

Yes, I replied to it and closed it.

@waldyrious
Copy link
Member

waldyrious commented May 6, 2024

Oh, I must have missed your response somehow — sorry about that. Great, let's go then! LGTM :)

@AdamWill AdamWill merged commit 52f63c7 into mwclient:master May 6, 2024
9 checks passed
@AdamWill
Copy link
Member Author

AdamWill commented May 6, 2024

ugh. So now I see we did actually do github releases from 0.7.1 onwards, and don't use the Changelog file any more. I guess I'll have to look into the github release bit of the upstream doc here after all, figure that out, and send a followup.

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