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
Wagtail 5 support #59
Conversation
html_template='wagtail_content_import/confirm_dialog.html', | ||
js_template=None, | ||
template_vars={}, |
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.
Unrelated change, just improves readability.
- Drop Python 3.7 from the matrix as it's unsupported - Drop Django 4.0 from the matrix as it's unsupported - Rename master to main (in line with Wagtail upstream)
3c715b6
to
7fb12c1
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.
Needs a couple of small changes. LGTM otherwise (caveat: haven't got a chance to test locally)
tox.ini
Outdated
python{3.8,3.9,3.10}-django{3.2}-wagtail{4.1,4.2,master}-{sqlite,postgres} | ||
python{3.8,3.9,3.10}-django{4.0,4.1}-wagtail{4.1,4.2,master}-{sqlite,postgres} | ||
python{3.11}-django{4.1}-wagtail{4.1,4.2,master}-{sqlite,postgres} | ||
python{3.8,3.9,3.10}-django3.2-wagtail{4.1,4.2,5.0,5.1,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.
Suggestion: drop Wagtail 4.2 as it has EOLed - https://endoflife.date/wagtail. This will simplify the matrix considerably
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, reshuffled this a bit in f45bfef, it's a little easier on the eyes/brain.
tox.ini
Outdated
django4.0: Django>=4.0,<4.1 | ||
django4.1: Django>=4.1,<4.2 |
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.
django4.0: Django>=4.0,<4.1 | |
django4.1: Django>=4.1,<4.2 | |
django4.1: Django>=4.1,<4.2 | |
django4.2: Django>=4.2,<5.0 |
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.
Fixed in 92e62bd
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.
Nitpick - the commit message says 4.1 (which isn't EOL yet as it's an LTS release), although it is indeed removing 4.2.
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, reworded.
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.
Other than the commit message nitpick, LGTM!
f45bfef
to
bfdd6f3
Compare
I have tested importing documents via:
I have tested constructing StructBlocks using custom converter classes.