Skip to content

Update tested versions in README#508

Open
martey wants to merge 1 commit into
jazzband:masterfrom
mobolic:update-readme-compatibility
Open

Update tested versions in README#508
martey wants to merge 1 commit into
jazzband:masterfrom
mobolic:update-readme-compatibility

Conversation

@martey

@martey martey commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

Update the Wagtail, Django, and Python versions mentioned as being tested in the README based on the tox configuration.

This section previously used the greater-than-or-equal sign (>=) when the less-than-or-equal sign (<=) was intended. Because these symbols are potentially confusing, they have been removed.

@MrCordeiro MrCordeiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is required, unfortunately. Most minor versions are backwards compatible and we are quick to update this package when they're not.

@MrCordeiro MrCordeiro closed this Feb 26, 2025
@martey

martey commented Feb 27, 2025

Copy link
Copy Markdown
Contributor Author

I agree with you that wagtailmenus is generally compatible with minor versions and that the testing section of the README does not need to include them. However, my changes are not related to minor versions and do not change how they are handled in this section.

I apologize for reopening this request, but the section is currently inaccurate. Right now, the README says that wagtailmenus is compatible with:

  • Wagtail versions greater than or equal to 5.2
  • Django versions greater than or equal to 5.1
  • Python 3.9 to 3.12

The section does not include Python 3.13. Because of the use of the greater than or equal signs that I previously mentioned, the section will automatically claim compatibility with future major versions of Django or Wagtail when they are released.

I tried to make a minimal amount of changes to fix these problems and thought that they were straightforward and non-objectionable, but maybe I should have created an issue to discuss this first. There are a number of potential alternate solutions to fix the current issues, in order of my personal preference:

  • removing the greater than or equal signs and adding Python 3.13 (these are the changes currently in this pull request)
  • changing the greater than signs to less than signs (since future major versions might have breaking changes that will require major changes)
  • adding Python 3.13 to the list of Python versions being tested (and ignoring the greater than or equal issue)
  • adding a greater than or equal sign to the list of Python versions being tested (so that Python 3.13 is added)

Please let me know if you would prefer one of these alternate solutions, and I will update this pull request to match it. If you don't think the current inaccuracy of the section is a problem, feel free to close this request again - I won't reopen it a second time.

@martey martey reopened this Feb 27, 2025
@MrCordeiro

Copy link
Copy Markdown
Contributor

Changing the greater than signs to less than signs (since future major versions might have breaking changes that will require major changes) and adding Python 3.13 to the list of Python versions being tested sounds like the ideal solution to me.

Plus, it keeps the testing matrix up-to-date, which we would appreciate!

Replace greater-than-or-equal signs with less-than-or-equal signs in the
version compatibility section of the README.
@martey martey force-pushed the update-readme-compatibility branch from 0b22c35 to c90274f Compare March 19, 2025 15:55
@martey

martey commented Mar 19, 2025

Copy link
Copy Markdown
Contributor Author

I've added a new commit that only changes the GTE signs to LTE signs, since #509 updated the Python version compatibility.

@MrCordeiro

Copy link
Copy Markdown
Contributor

Hello @martey ,
The review request was to change it to LT a major version, not LTE a minor version. I.e.: Wagtail versions < 6, Django < 6

@martey

martey commented Sep 25, 2025

Copy link
Copy Markdown
Contributor Author

Hi,

Sorry for the delay in getting back to you.

The review request was to change it to LT a major version, not LTE a minor version.

I know that this seems simple and not that different from my existing changes, but I think that it actually makes the section harder to understand and less accurate.

Neither Django nor Wagtail use true semantic versioning, so all new versions are "mostly backwards-compatible with the previous release" but not entirely backwards-compatible (see https://docs.djangoproject.com/en/6.0/internals/release-process/ and https://docs.wagtail.org/en/v7.1/releases/release_process.html). Because of this, I don't think we should assume that a version of wagtailmenus that is compatible with Django or Wagtail X.0 will be compatible with version X.1, much less a later release like X.2 or X.3.

I should apologize because my previous comment wasn't completely clear - I used "less than signs" as a shorthand for LTE signs, and "major versions" as a shorthand for Django/Wagtail "feature release" versions. By "minor versions", I meant what Django and Wagtail call "patch releases", which should have full backwards compatibility.

If you still would like me to change my PR to use LT signs (instead of LTE signs) and "X.0" versions (instead of feature release versions), let me know and I will update it.

@MrCordeiro MrCordeiro closed this Oct 7, 2025
@jazzband jazzband locked and limited conversation to collaborators Oct 7, 2025
@martey

martey commented Oct 7, 2025

Copy link
Copy Markdown
Contributor Author

I don't understand why this was closed and locked. I don't think I have been disrespectful or disruptive.

@jazzband jazzband unlocked this conversation Oct 10, 2025
@MrCordeiro MrCordeiro reopened this Oct 10, 2025
@MrCordeiro

Copy link
Copy Markdown
Contributor

@martey , closed this by mistake. Apologies!

Although I believe < are clearer, that's just my personal choice.
Your branch is out-of-date. Please update it so we get rid of the merge conflicts.

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