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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modernise for latest Wagtail and Django #474

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

nickmoreton
Copy link
Collaborator

Working with Wagtail 4.1

This is a rework of the Longclaw master branch that adjusts the code for working with Wagtail 2.15+ but makes a few other changes, for now, to hopefully re-boot a Wagtail package that always looked so promising.

I am happy to hear any feedback 馃槃

What is fixed

It's opinionated but...

  • It requires Wagtail 2.15+ (LTS) and Django 3.2+ (LTS)
  • The original tests run OK.
  • Tox is modernised and runs in github actions, along with coverage.
  • I think I have made the test app work better for a developer.
  • Developer setup is improved without been too different.
  • The admin site works well, especially nice with Wagtail 4.1
  • The orders Dashboard is moved to it's own admin page, at least for the moment.

What not fixed / removed / broken

Although I haven't included some original code there's no reason It couldn't be added back in.

  • Not fixed, frontend client side CSS/JS
  • The templates could do with some work.
  • The project_template isn't included now.
  • The documentation needs attention, mainly to update it to the latest Wagtail version.
  • Shopping basket
  • Checkout
  • And may be something else you see and fancy getting involved in!

nickmoreton and others added 27 commits November 13, 2022 15:14
Remove python 3.7
I have ignore the help_text for flake8 because extra migrations
are required if you format it.
I am seeing errors sometimes with longclaw.test.tests.test_shipping.ShippingOptionEndpointTest
I wonder why they have appeared again?
There's some temporary fixes in place here until I can
confirm all the features are working as expected.
Add codyhouse css and js cdn
@nickmoreton
Copy link
Collaborator Author

I've added GitHub Actions but have seen random failures with loading the countries there. It doesn't happen locally.

@JamesRamm
Copy link
Collaborator

This is a big PR.
In theory I would be happy for it to be merged, as it is important, but the issues would need to be fixed first.
I.e you have pointed out a few things that are in a broken state. The project_template is fairly important just because the entire documentation directs you to create a new shop using this, so if that is broken no-one will be able to on-board.

If it is tricky to fix all issues in this PR, I would suggest trying to break it up into more atomic updates. E.g one PR for updating wagtail, one PR for modernising tox and github actions and so on. 7

This will make it much easier to review and merge

@nickmoreton
Copy link
Collaborator Author

Thanks @JamesRamm Yep it's a big one 馃槃 you could say I got carried away but I think it shows the way forward.

I'll find some time to break it down into smaller pieces of work so it can be reviewed easier and merge in bit by bit.

@JamesRamm
Copy link
Collaborator

@nickmoreton do you feel like going further with this PR?

@nickmoreton
Copy link
Collaborator Author

@nickmoreton do you feel like going further with this PR?

Hi @JamesRamm Yes I would. I鈥檝e not had time to work on it since the PR due to other work. I鈥檒l see what I can do to break it down into smaller pieces of work.

@nickmoreton nickmoreton mentioned this pull request May 3, 2023
@nickmoreton
Copy link
Collaborator Author

Hi @JamesRamm I made a start: #485

What do you think? Thanks

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

2 participants