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

fix: add wagtail-modeladmin as dependency #470

Closed
wants to merge 2 commits into from

Conversation

JanMalte
Copy link

this is just a quick fix for #459

The project should migrate to use wagtail snippets instead.

according to the contribute guidelines, I've added myself to the list
Copy link
Contributor

@MrCordeiro MrCordeiro left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @JanMalte!

wagtail-modeladmin is already listed as a development-extra requirement. Could you remove it from there? It's doesn't make sense to have it listed twice...

(PS I saw the tests failed... but I don't think it's due to your PR. It's already late over here, so I'm going to give it a better look tomorrow)

@dkirkham
Copy link
Contributor

dkirkham commented Dec 1, 2023

When I prepared #461 I didn’t add the new non-contrib version of ModelAdmin as a deployment requirement because if an older version of Wagtail was being used you would end up with both the contrib and non-contrib versions loaded; it seemed that delegating that choice to the developer made sense, and the installation documentation aligns with this.

Before hard-coding non-contrib ModelAdmin as a deployment requirement, it should be tested against pre-5.1 versions of Wagtail.

By February 2024 Wagtail 4.1 LTS will go out of support (see https://endoflife.date/wagtail) so I suggest that is the time to add non-contrib ModelAdmin as a deployment requirement, maybe as a minor or major release.

@MrCordeiro
Copy link
Contributor

Hello! With #480, this PR will no longer be necessary. I'll be closing it for now. Still, @JanMalte , your help was appreciated!

@MrCordeiro MrCordeiro closed this Feb 26, 2024
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.

3 participants