-
Notifications
You must be signed in to change notification settings - Fork 5
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 GHA CI configuration #30
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks! This is pretty cool. I added a couple of comments to hopefully simplify/improve some stuff.
- python: python3.8 | ||
dist: ubuntu:focal | ||
#- python: python3.10 | ||
# dist: ubuntu:jammy |
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 error you had mentioned looks like this: mosquito/cysystemd#36
A comment there recommends doing pip install cysystemd
instead, and indeed the PyPI page for systemd
actually links to the cysystemd
repository. So maybe changing that in run.sh
would make things work?
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.
Maybe? I'd say that is a separate change. My goal here is just to port the existing CI, not to improve/update the things we test. :) I had to change to newer Ubuntu releases since the old ones are no longer available or stopped working with recent pip packages, but I don't plan to do any more here than is necessary to maintain the old level of test coverage.
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Pull docker container | ||
run: docker pull ${{ matrix.dist }} |
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.
Is there really a good reason to use Docker manually at this point? If I remember correctly, that was mainly a Travis CI thing? Couldn't we just use runs-on: ${{ matrix.dist }}
above and run the script directly instead (probably after running setup-python).
Btw, to try those things out locally, there's act, in case you weren't aware of it yet.
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.
Good question. As I just wrote in the other thread, I wanted to do the bare minimum of work to get CI green again. So this was the easiest thing to do. Feel free to simplify the setup in follow-up changes. :)
Fixes #26
I also bumped the Ubuntu versions we test against to not be quite so ancient. Note that the latest Ubuntu LTS (22.04) fails, so it is commented out. It shows this error while "Running setup.py install for systemd":