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

Support/wagtail42 #38

Closed
wants to merge 3 commits into from
Closed

Support/wagtail42 #38

wants to merge 3 commits into from

Conversation

nickmoreton
Copy link

@nickmoreton nickmoreton commented Feb 7, 2023

This updates the package for wagtail 4.2 support.

Adds:

  • Testing for wagtail >= 4.1

Removes:

  • Support for wagtail < 4.1
  • Testing for wagtail < 4.1

@cnk
Copy link
Collaborator

cnk commented Feb 7, 2023

Thanks @nickmoreton! Once I get the release done for versions <= 3.0 I will merge this.

I am holding off on that release in hopes that someone can figure out #32 which affects all versions of wagtail-hallo. Then we can merge this PR and the one that fixes the document and image choosers for Wagtail 4+ #33

Comment on lines +6 to +8
python{3.7}-django{3.2}-wagtail{4.1,4.2,main}-{sqlite,postgres}
python{3.8,3.9,3.10}-django{4.0,4.1}-wagtail{4.1,4.2,main}-{sqlite,postgres}
python{3.11}-django{4.1,main}-wagtail{4.1,4.2,main}-{sqlite,postgres}
Copy link
Contributor

@katdom13 katdom13 Feb 9, 2023

Choose a reason for hiding this comment

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

Suggestion:

  • Create two postgres environments
  • Test different combinations of python, django and wagtail
envlist =
    python{3.7}-django{3.2}-wagtail{4.1,4.2}-{sqlite,postgres12}
    python{3.8,3.9,3.10}-django{3.2}-wagtail{4.1,4.2}-{sqlite,postgres12}
    python{3.8,3.9,3.10}-django{4.0,4.1}-wagtail{4.1,4.2}-{sqlite,postgres15}
    python{3.11}-django{4.1}-wagtail{4.1,4.2}-{sqlite,postgres15}

deps =
    ...

    postgres12: psycopg2>=2.6,<2.9.5
    postgres15: psycopg2>=2.9.5

As suggested from this conversation: https://torchbox.slack.com/archives/C03R0FNK7/p1675655322697539
Reference: https://github.com/wagtail/wagtail-localize/pull/673/files#r1097252879

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently, newer versions of postgres (15+) work with Python 3.11 but not with older Django (3.2); older versions of postgres (10 - 12) do not work with Python 3.11 but work with Django 3.2.
So tests with Django 3.2 are isolated.

@katdom13
Copy link
Contributor

katdom13 commented Feb 9, 2023

Hi @nickmoreton ,
Left a few comments for your consideration, but this LGTM.
Thanks!

@jorenham
Copy link
Contributor

Bump :)

@jozo
Copy link

jozo commented Aug 14, 2023

Hi, I've forked code from #39 and (hopefully) fixed #33 . Can you please check it and merge it? @cnk
It's in branch fix-toolbar-in-wagtail4 - https://github.com/jozo/wagtail-hallo/tree/fix-toolbar-in-wagtail4

@nickmoreton nickmoreton closed this by deleting the head repository Oct 6, 2023
@jorenham
Copy link
Contributor

@nickmoreton why did you delete your fork?

@nickmoreton
Copy link
Author

@nickmoreton why did you delete your fork?

Ah that was me doing some house keeping. I was doing this work as part of a larger project including many other package updates. We're now using a new organisation on GitHub to carry out the work. We're updating packages used on client sites and this was no longer used by us. I will see if i can get a local copy back in place and possibly set it up again if it's something you need. I am out of the office till next week but will have a look asap
Sorry if this has caused problems for you. 😕

@jorenham
Copy link
Contributor

@nickmoreton no worries, see #40

@nickmoreton
Copy link
Author

nickmoreton commented Oct 16, 2023

@jorenham I do have a local copy still if you need anything from it but it looks like your PR includes the changes. 👍

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.

5 participants