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

[7637] django upgrade to 4.2 #2552

Merged
merged 2 commits into from
Apr 10, 2024
Merged

[7637] django upgrade to 4.2 #2552

merged 2 commits into from
Apr 10, 2024

Conversation

m4ra
Copy link
Contributor

@m4ra m4ra commented Feb 5, 2024

To be merged after wagtail upgrade #2554

Tasks

  • PR name contains story or task reference
  • Documentation (docs and inline)
  • Tests (including n+1 and django_assert_num_queries where applicable)
  • Changelog

@m4ra m4ra force-pushed the mk-2024-02-django-upgrade-4.0 branch from 1cc15d5 to 7300abf Compare February 5, 2024 15:27
@m4ra m4ra requested a review from goapunk February 5, 2024 15:27
@m4ra
Copy link
Contributor Author

m4ra commented Feb 6, 2024

@goapunk there is an issue with the background task app. So I will rebase this PR once you merge #2542 into main.

@m4ra m4ra force-pushed the mk-2024-02-django-upgrade-4.0 branch from 7300abf to 35eb721 Compare February 6, 2024 16:29
@m4ra m4ra changed the title WIP [7637] django upgrade to 4.0 [7637] django upgrade to 4.0 Feb 6, 2024
Copy link
Contributor

@goapunk goapunk left a comment

Choose a reason for hiding this comment

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

Nice, looks good, just one thing which I think is missing: the delete() -> form_valid() change was part of this update already

@m4ra m4ra force-pushed the mk-2024-02-django-upgrade-4.0 branch from 35eb721 to 294c980 Compare February 7, 2024 14:43
@m4ra m4ra requested a review from goapunk February 7, 2024 14:44
@m4ra m4ra force-pushed the mk-2024-02-django-upgrade-4.0 branch 2 times, most recently from fd2c1e8 to a6f9f0a Compare February 7, 2024 15:09
@m4ra m4ra changed the title [7637] django upgrade to 4.0 [7637] django upgrade to 4.2 Feb 8, 2024
Copy link
Contributor

@goapunk goapunk left a comment

Choose a reason for hiding this comment

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

Really nice! Only thing we need to take into account is that by updating the a4 hash we now have the ckeditor5 changes, so we can't release this until we did the ckeditor5 aplus story

@m4ra
Copy link
Contributor Author

m4ra commented Feb 22, 2024

@goapunk or we don't update the a4 hash.

@goapunk
Copy link
Contributor

goapunk commented Feb 22, 2024

@goapunk or we don't update the a4 hash.

will it cause problems with a4 on django 3.2 and a+ on django 4.2 if we don't update the hash?

@m4ra
Copy link
Contributor Author

m4ra commented Feb 22, 2024

@goapunk the topics might be an issue, though we don't use it in a+. Need to check if ckeditor was added after the multiselect-field.

@m4ra m4ra force-pushed the mk-2024-02-django-upgrade-4.0 branch 2 times, most recently from 24e42f9 to 0fe1c33 Compare April 2, 2024 15:08
@m4ra m4ra force-pushed the mk-2024-02-django-upgrade-4.0 branch from 50041ff to 7239e86 Compare April 9, 2024 15:27
@m4ra m4ra requested a review from goapunk April 9, 2024 15:27
@m4ra m4ra force-pushed the mk-2024-02-django-upgrade-4.0 branch from 7239e86 to 177e0ca Compare April 9, 2024 15:34
Copy link
Contributor

@goapunk goapunk left a comment

Choose a reason for hiding this comment

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

LGTM, we didn't do the deprecated app config bit but we can do them in a new PR and get this merged I'd say

@goapunk goapunk merged commit 5300256 into main Apr 10, 2024
2 of 3 checks passed
@goapunk goapunk deleted the mk-2024-02-django-upgrade-4.0 branch April 10, 2024 12:35
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