RFC 74: Core module reorganisation #74
Conversation
| - `wagtail/admin/templatetags` will be merged into `wagtail/templatetags` | ||
| - `wagtail/admin/static_src` will be moved to `wagtail/static_src` | ||
| - `wagtail/admin/locale` will be merged into `wagtail/locale` | ||
| - `wagtail/admin/wagtail_hooks.py` will be moved to `wagtail/wagtail_hooks/admin.py` temporarily. They will be merged with Wagtail core hooks in a later step. We may have to split this module into a sub-folder because it’s getting quite big. |
There was a problem hiding this comment.
I'm thinking a long term solution here could be to split that main wagtail_hooks.py file up by functionality rather than app:
wagtail_hooks/rich_text.pywagtail_hooks/page_actions.pywagtail_hooks/log_actions.pywagtail_hooks/main_menu.py
There was a problem hiding this comment.
I like this idea. 👍
I'm not 100% sure, but does the INSTALLED_APP order also dictate the order hooks are called?
Reworking might have nasty side-effects. Not that we don't want to refactor. It is just something to be aware of.
text/074-module-reorganisation.md
Outdated
| - [`compat.py`](https://github.com/wagtail/wagtail/blob/f2f4503f4ffb1130e770a3dd4d98ef6e47864de3/wagtail/core/compat.py) | ||
| - [`treebeard.py`](https://github.com/wagtail/wagtail/blob/8f8f2e12b731334bbe1e39508a12246a3503f532/wagtail/core/treebeard.py) | ||
| - [`url_routing.py`](https://github.com/wagtail/wagtail/blob/8e25960972a1663b14faca1f7ac6a1693a581b18/wagtail/core/telepath.py) | ||
| - [`whitelist.py`](https://github.com/wagtail/wagtail/blob/ba6f94def17b8bbc66002cbc7af60ed422658ff1/wagtail/core/whitelist.py) |
There was a problem hiding this comment.
Can we rename this to allowed_list?
There was a problem hiding this comment.
I like this idea! The class that this module contains is called Whitelister though so I think we should come up with a new name for that too
|
|
||
| ## Move `wagtail/core` to `wagtail` | ||
|
|
||
| This will be carried out by moving all modules currently within `wagtail/core/` up to `wagtail/` then updating all imports and docs. We will add dummy modules into `wagtail/core` that re-export anything that may have been previously imported from there (such as the app config, blocks and models). |
There was a problem hiding this comment.
I recall we had made similar module relocations as part of Wagtail 2.0, as a breaking change. Would it be worthwhile to similarly consider implementing this RFC as a breaking change, rather than adding dummy-modules with re-exports?
I have some concern about users importing undocumented / private things from Wagtail modules as well. Is this something we need to be worried about?
| - `wagtail.embeds` | ||
| - `wagtail.snippets` | ||
|
|
||
| This also gives the `wagtail.contrib` section a clear purpose as all apps that are not in this folder are compulsory. |
There was a problem hiding this comment.
Just a thought - if the compulsory apps are going into wagtail, do we actually need to move these non-compulsory ones into contrib, since there'll no longer be any ambiguity about which ones are compulsory or not?
I suspect that most devs probably consider images / docs / snippets to be 'core' functionality of Wagtail (regardless of whether that's technically true), and would see no reason to drop them from a project - to them, all this change does is add extra typing.
There was a problem hiding this comment.
I was thinking that Django has contrib apps that are used on virtually every site (such as, django.contrib.sessions) and was trying to follow that.
It's nice in some ways to make everything super-consistent, but this change is probably going to be annoying for people to switch to and I agree that there is very little to gain from it once our "compulsary" apps are merged into wagtail so I think we should leave this bit out.
| - [`compat.py`](https://github.com/wagtail/wagtail/blob/f2f4503f4ffb1130e770a3dd4d98ef6e47864de3/wagtail/core/compat.py) | ||
| - [`treebeard.py`](https://github.com/wagtail/wagtail/blob/8f8f2e12b731334bbe1e39508a12246a3503f532/wagtail/core/treebeard.py) | ||
| - [`url_routing.py`](https://github.com/wagtail/wagtail/blob/8e25960972a1663b14faca1f7ac6a1693a581b18/wagtail/core/telepath.py) | ||
| - [`whitelist.py`](https://github.com/wagtail/wagtail/blob/ba6f94def17b8bbc66002cbc7af60ed422658ff1/wagtail/core/whitelist.py) -> should be renamed to `allowlist` or `safelist` and then `Whitelister` module renamed to `Gatekeeper` or `Safelister` |
There was a problem hiding this comment.
I think it probably makes sense to move this file under wagtail/rich_text (I think this folder was added much later than whitelister.py which is probably why it's not there right now).
What do you think?
|
This is partly implemented |
This RFC contains a proposal for a reorganisation of Wagtail's core modules. There's a few reasons why we need to do this:
.coresuffix from core modules and_tagsfrom template tagsAll of these changes will be backwards compatible.
The reorganisation described in this RFC is already been implemented in the restructure branch. This was developed using a script to make the process repeatable on future Wagtail versions.
This branch is available as a
pypipackage:wagtail-restructureRendered