-
Notifications
You must be signed in to change notification settings - Fork 11
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 precommit and update tox targets #28
Conversation
cc @allcaps |
…existing jobs on new commits
- updates Python, Django and Wagtail targets - adds interactive mode - allows passing args to tox
contains notes about pre-commit and tox args
86a4554
to
db009d5
Compare
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 is going to have mad merge conflicts with #32 but I reckon lets get this one finished, merge it roughly as it stands, and then I'll rebase my branch onto this, update all the pip/setuptools stuff to flit.
One of your finest reviews would be appreciated in return 🙏
If you want to kick the can of python 3.11/wagtail 4.1 down the road, I reckon that's reasonable.
python{3.7,3.8,3.9}-django{3.2}-wagtail{2.15,3.0,4.0}-{sqlite,postgres} | ||
python{3.10}-django{3.2}-wagtail{2.15,3.0,4.0,main}-{sqlite,postgres} | ||
python{3.7,3.8,3.9,3.10}-django{4.0}-wagtail{3.0,4.0,main}-{sqlite,postgres} | ||
python{3.8,3.9,3.10}-django{4.1,main}-wagtail{4.0,main}-{sqlite,postgres} |
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.
Python 3.11 has been released in the time between submitting this and someone reviewing it, and wagtail 4.1. Shall we get both tox and the workflow file testing on the latest versions, or just get this merged and then do another Pr bumping them. I don't want to let perfect be the enemy of the good too much here..
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.
added to the mix
cancel-in-progress: true | ||
|
||
permissions: | ||
contents: read # to fetch code (actions/checkout) |
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.
Some comment as #29 (comment) here. This is now the default
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.
Leaving it there on the "explicit > implicit"
wagtail-{{ cookiecutter.project_name_kebab }}/.github/workflows/test.yml
Show resolved
Hide resolved
Co-authored-by: chris48s <[email protected]>
Ta. I'm going to merge this one now. I will deal with the monster rebase on #32 sometime after Christmas. |
Thanks @zerolab and all - this is a nice improvement |
Fixes #15
Fixes #21
the eslint job should be added after #20