Support wagtail42 - #61
Conversation
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
There was a problem hiding this comment.
Cancels previous running actions if not finished.
| - id: black | ||
| args: ["wagtailsharing", "setup.py", "--line-length=79"] | ||
| exclude: migrations | ||
| - repo: https://gitlab.com/pycqa/flake8 | ||
| - repo: https://github.com/pycqa/flake8 |
There was a problem hiding this comment.
gitlab url is now private
| additional_dependencies: [flake8-bugbear==22.1.11] | ||
| - repo: https://github.com/pycqa/isort | ||
| rev: 5.10.1 | ||
| rev: 5.12.0 |
There was a problem hiding this comment.
isort was erroring when pre-commit ran
| def route(self, request, path_components): | ||
| if getattr(request, "routed_by_wagtail_sharing", False): | ||
| page = self.get_latest_revision_as_page() | ||
| page = self.get_latest_revision_as_object() |
There was a problem hiding this comment.
Changed in Wagtail 4.0
|
|
||
| # Get the latest revision for the requested page. | ||
| page = page.get_latest_revision_as_page() | ||
| page = page.get_latest_revision_as_object() |
There was a problem hiding this comment.
Changed in Wagtail 4.0
victoriachan
left a comment
victoriachan
left a comment
There was a problem hiding this comment.
Hi Nick, I have reviewed this. It looks good. The Readme needs updating though. I have attached some new screenshots to save you the trouble.
I wasn't sure if the Page.serve() changes would affect this, so I tried this locally and it works fine.
| @@ -202,9 +202,9 @@ Compatibility | |||
|
|
|||
| This project has been tested for compatibility with: | |||
There was a problem hiding this comment.
Further up on you missed The code examples below assume that you are using the latest Wagtail version (3.0+).. Also wondering if all the screenshots need to be replaced to reflect the look and feel of Wagtail 4.1+.
Since I have been trying this locally, I have taken a few screenshots to save you the trouble:
images.zip
There was a problem hiding this comment.
Thanks @victoriachan I have updated the docs.
And thanks so much for the screen shots too 👍
Thank you Victoria for the new screenshots!
|
Looks good to me! |
|
Is there a reason this PR is also bringing in token URLs? That seems like a larger change, and it'd be nice to either update #47 or propose that change separately from a version bump, keep the supported version bump more tightly scoped. |
|
@nickmoreton It looks like your upgrade commits are relatively independent of the token commits. I'll pull them into a separate branch and get it ready to merge! |
Sorry @willbarton I missed your reply here. That would be great to use the upgrade part separately. Thanks |
|
@nickmoreton I'm closing this as support for Wagtail versions up through 4.2 has now been added in #62, based in part on your contributions. Thank you! |
5 participants
This updates the package for Wagtail 4.2
Additions
Wagtail 4.1
Removals
Wagtail < 4.1
Changes
Testing
Updates github actions and tox matrix
Review
Preview this PR without the whitespace changes
Screenshots
Notes
Todos
Checklist