kvas-it / pytest-console-scripts

Pytest plugin for testing console scripts
MIT License
78 stars 14 forks source link

Automate releases via GitHub actions #76

Closed kvas-it closed 1 year ago

kvas-it commented 1 year ago

A first shot at automated release action (written by ChatGPT with minor corrections). Looks about right?

codecov[bot] commented 1 year ago

Codecov Report

Merging #76 (21aeeb2) into master (db5bf30) will increase coverage by 0.05%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
+ Coverage   99.53%   99.58%   +0.05%     
==========================================
  Files           1        1              
  Lines         214      242      +28     
==========================================
+ Hits          213      241      +28     
  Misses          1        1              

see 1 file with indirect coverage changes

kvas-it commented 1 year ago

Thanks for the feedback. Some quick comments.

The actions used are outdated.

Yeah, I guess ChatGPT doesn't know about the latest versions ;)

  • This skipped building wheels.

The original code from ChatGPT actually had wheel building, I just removed it because with my existing workflow I didn't build wheels. My understanding is that for a pure Python package wheels don't offer much of an advantage compared to a source package, but perhaps it's still worth it.

  • Do not add your PyPI password to GitHub's secrets. There are special API tokens to use for this and since you have your real username here it seems like you're not using tokens correctly.

Yeah, I know that an API token should be used, I just didn't generate it yet, so I wasn't sure how exactly it's supposed to be used.

Alright, I will see how to improve on this.

HexDecimal commented 1 year ago

My understanding is that for a pure Python package wheels don't offer much of an advantage compared to a source package, but perhaps it's still worth it.

ChatGPT was correct to make wheels. Wheels are faster to install than a source distribution. They skip the build dependencies and setup.py script and extract the built files directly where they need to go.

Yeah, I know that an API token should be used, I just didn't generate it yet, so I wasn't sure how exactly it's supposed to be used.

PyPI should have docs where you can generate the tokens. The username becomes __token__ and the password is the API key which needs to be kept secret.

I'd do it myself, but I'm not an owner of the PyPI package so I can't set up tokens or trusted publishing. Even if I was the owner I can't set up the environment recommended by trusted publishing since I don't own the repo and the repo can't be co-owned unless it's part of an organization. The documentation for those features is pretty good so you'll just have to do a but of reading to set it up yourself.

If you like I can push my other suggested changes to this PR.

This PR also needs to be rebased into master. You forked this branch from before we fixed the CI.

kvas-it commented 1 year ago

I'd do it myself, but I'm not an owner of the PyPI package so I can't set up tokens or trusted publishing. Even if I was the owner I can't set up the environment recommended by trusted publishing since I don't own the repo and the repo can't be co-owned unless it's part of an organization. The documentation for those features is pretty good so you'll just have to do a but of reading to set it up yourself.

I have set up the token already and added it to the repo secrets.

This PR also needs to be rebased into master. You forked this branch from before we fixed the CI.

Done.

If you like I can push my other suggested changes to this PR.

If you have time, feel free to do it. I probably won't get to working on this today, but otherwise can finish later.

HexDecimal commented 1 year ago

Since merges with master already require reviews I didn't make it a priority to make the jobs defendant. You can add needs: [tests] to the publish job if you want it to come only after tests pass.

HexDecimal commented 1 year ago

Here's the trusted publishing setup. This requires the releases workflow and needs to be enabled on PyPI or else it will likely fail.

This can be changed to not use an environment, or it can even be changed to use just the API token like the Twine setup, but the current setup right now is the recommended one.

kvas-it commented 1 year ago

Thanks for bringing this project to the cutting edge! ;)

I have set up "release" environment (limited to "master" branch). I have also enabled publishing from this workflow and environment on PyPI. Shall we merge and try at the next release?

HexDecimal commented 1 year ago

Everything looks correct. You should try and make a 1.4.1 release with the current bug fixes.

kvas-it commented 1 year ago

Perhaps we can make a release when Python 3.7 reaches EOL. Although the actual EOL date doesn't really matter that much: 3.7 users just won't see the new version and everyone else will benefit 🤔

I guess we can also just do it now. BTW, I really like the improved changelog and it's Unreleased section that makes it really easy and quick to see what you're releasing.

HexDecimal commented 1 year ago

Don't let older Python versions hold you back. I at least made sure to get my new features in before dropping it. As someone who wrote a lot of Python 2/3 code it's usually too much effort to backport stuff for what you get out of it.

The Keep A Changelog style is pretty good. I use it for all of my projects. Be sure to read the site if you haven't.

I don't see a reason to sit on these bug fixes. I'll make a PR to prepare a new release.