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

Wagtail 4 #111

Merged
merged 6 commits into from
Nov 26, 2022
Merged

Wagtail 4 #111

merged 6 commits into from
Nov 26, 2022

Conversation

katdom13
Copy link

Release notes: https://docs.wagtail.org/en/stable/releases/4.0.html

  • Updated docs
  • Updated some imports on test

Tests are OK

Found 14 test(s).
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
..............
----------------------------------------------------------------------
Ran 14 tests in 0.130s

OK

@nickmoreton
Copy link

Hey @katdom13 Just one thing I noticed in the docs.

body = StreamField([
        ('map', LeafletBlock()),
    ])

for correctness should be

body = StreamField([
        ('map', LeafletBlock()),
    ], use_json_field=True)

@katdom13
Copy link
Author

Hi @nickmoreton ,
I have fixed the doc as here:
e360d10

Thank you for this!

@nickmoreton
Copy link

I have fixed the doc as here:
e360d10

I think there are a few more that can be updated, not exactly the same as the above. Be good to get them all updated 😄

Copy link

@nickmoreton nickmoreton left a comment

Choose a reason for hiding this comment

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

There's a couple of places I noted an improvement to reduce the code duplication.

tests/geopage/models.py Outdated Show resolved Hide resolved
tests/geopage_nospatial/models.py Outdated Show resolved Hide resolved
tests/geopage/models.py Outdated Show resolved Hide resolved
Copy link

@nickmoreton nickmoreton left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @katdom13

@marteinn
Copy link
Member

Thank you @katdom13 for the fantastic work and @nickmoreton for reviewing. LGTM, merging.

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.

None yet

3 participants