-
-
Notifications
You must be signed in to change notification settings - Fork 137
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
🔖 (release): v4.0.0rc1 #480
Conversation
Hi @schlich, thanks for merging #472, but it wasn't quite ready... as well as the docs, there are some packaging and testing changes. Many are in my local repo, so I'll now create separate PRs for those. There are also some breaking changes, impacting anyone who has customised the admin, which would suggest we should be going for v4.0 rather than v3.2.0. |
Thanks for the update @dkirkham -- the merge doesn't trigger a release or anything, and as far as I'm concerned as a maintainer if it passes CI it is safe to merge to main -- if you think the merge is in a broken state then let's make sure we capture those failure states in tests. Thanks for the major release warning -- I had questions about whether this might be a minor or major release. Can you point me to some code related to the breaking change? Let's make sure we have a migration process documented. Let me know if there's anything I can do to help with the packaging/testing changes you mention (maybe this resolves my comments above). |
On that note, @schlich please allow at least one day between opening a PR and merging it so that it gets the chance to be reviewed by other people. This is what we've been following in this project for other PRs. |
@schlich, see the comment at #472 (comment). I have mostly documented this breaking change in the release notes, which I haven’t had a chance to commit - a bit more checking to do first. |
wagtailmenus/__init__.py
Outdated
@@ -2,7 +2,7 @@ | |||
|
|||
# major.minor.patch.release.number | |||
# release must be one of alpha, beta, rc, or final | |||
VERSION = (3, 1, 9, "final", 0) | |||
VERSION = (3, 2, 0, "rc", 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.
VERSION = (3, 2, 0, "rc", 0) | |
VERSION = (4, 0, 0, "rc", 0) |
I think we can call this v4.0... By the way, don't forget to update the CHANGELOG.md
and the documentation's release notes in this PR 😉
https://wagtailmenus.readthedocs.io/en/stable/releases/index.html
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.
May I suggest the following CHANGELOG entry for my recent contribution:
* Fix [#459](https://github.com/jazzband/wagtailmenus/issues/459) by adding support for Wagtail 5.2, Wagtail 6.0 and Python 3.12.
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.
I would just go with "Added support for x, y, z" and then add another entry saying
Fix [#459](https://github.com/jazzband/wagtailmenus/issues/459) by migrating to wagtail.snippets. Removed `wagtail_modeladmin` support.
That way it becomes more explicit.
@dkirkham, I think we are close to a release, right? :D |
@MrCordeiro and @schlich, I'm happy with my changes, now that they are merged. I have retested with a full set of tests using As far as I can see, this PR needs to have:
May I leave that to you or the other contributors? |
Yep i can take care of that this evening, rock on |
No description provided.