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

Support wagtail 2.16 #152

Merged
merged 5 commits into from
Feb 14, 2022

Conversation

fabienheureux
Copy link
Contributor

@fabienheureux fabienheureux commented Feb 9, 2022

Support wagtail 2.16

Wagtail 2.16 is now out, this PR adds support for this new version.

Note : I added wagtail 2.15 and 2.16 to github actions as we currently only test with wagtail 2.14.
Is there a reason these versions were not tested in the CI ?


  • Testing
    • CI passes
    • These changes do not reduce test coverage
  • Documentation.
    • This PR adds or updates documentation

@fabienheureux
Copy link
Contributor Author

fabienheureux commented Feb 9, 2022

Note for later : we might want to define the matrix inside a json file and load it using fromJson in the github action, or use something like https://tomasvotruba.com/blog/2020/11/16/how-to-make-dynamic-matrix-in-github-actions/ as it would prevent the duplicated block in the workflow file

@@ -45,8 +48,11 @@ jobs:
matrix:
python: ['3.8', '3.9']
django: ['3.1', '3.2']
wagtail: ['2.14']
wagtail: ['2.14', '2.15', '2.16']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question : is it worth testing against all wagtail versions here ? Or is it safe to assume it is already tested against them in the previous job ?

@nickmoreton
Copy link
Collaborator

Thanks @fabienheureux

Note : I added wagtail 2.15 and 2.16 to github actions as we currently only test with wagtail 2.14.
Is there a reason these versions were not tested in the CI ?

Not that I can think of. It's likely we just haven't caught up yet, pretty sure 2.15 was the current release when we started the project so not sure why we didn't have that in there.

@nickmoreton
Copy link
Collaborator

@fabienheureux Thanks for the PR. LGTM. I will merge this in and get a new release in place today.

Note for later : we might want to define the matrix inside a json file and load it using fromJson in the github action, or use something like https://tomasvotruba.com/blog/2020/11/16/how-to-make-dynamic-matrix-in-github-actions/ as it would prevent the duplicated block in the workflow file

Will add your note to a new issue and we can continue the discussion there. 👍

@nickmoreton nickmoreton merged commit df7daf6 into torchbox:main Feb 14, 2022
@fabienheureux
Copy link
Contributor Author

Thanks for the merge, regarding my note I already created an issue to track this #153

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.

2 participants