[17.0] [IMP] storage: Remove dependency on server_environment - #654
[17.0] [IMP] storage: Remove dependency on server_environment#654yankinmax wants to merge 18 commits into
Conversation
5077001 to
959ad04
Compare
|
Hello @etobella @lmignon @ivantodorovich |
abb1082 to
04b1706
Compare
| "depends": ["fs_storage", "server_environment"], | ||
| "post_init_hook": "post_init_hook", | ||
| "uninstall_hook": "uninstall_hook", | ||
| "auto_install": True, |
There was a problem hiding this comment.
Hmm are we sure we want to make these modules auto-install?
That means that if I still have server env installed for a different app (eg: mail) this module will be installed as well w/o any reason.
Since the migration is taking care of installing the modules I would say this has to change.
Am I missing any particular reason to keep auto-install?
There was a problem hiding this comment.
The reason was to keep it as much as possible similar to what it was before, so making them auto install would prove the goal, but there is no specific reason.
WDYT @ivantodorovich @lmignon @etobella ?
There was a problem hiding this comment.
For the next version of Odoo, we should remove the autoinstall. However, since this feature was enabled by default in existing versions, I think we should preserve the current default behaviour for backward compatibility.
BTW, I recently discovered the root cause of a strange behaviour with server.env.mixin, where some fields can no longer be edited manually. see OCA/server-env#289
There was a problem hiding this comment.
Finally, I've decided to remove auto_install and tend to agree with @simahawk :
All five upgrade scripts explicitly call button_install.
Removing auto_install:
- Preserves environment managed fields and previous behavior during upgrades.
- Prevents fresh installations from unexpectedly installing a
storage_environment_*module just becauseserver_environmentis used a dependency by another module. - The
auto_installhas never landed to recent versions actually. It was an idea to add it, but it seems there is no practical need.
|
The dependency is getting merged 😉 |
…lumns on uninstall
… columns on uninstall
…ore columns on uninstall
…re columns on uninstall
…restore columns on uninstall
I'll update this PR without |
04b1706 to
ca9cde7
Compare
|
@yankinmax ready for merge w/ nobump, right? |
Yep, the version is bumped manually accordingly with upgrade script |
|
/ocabot merge nobmup |
|
Hi @lmignon. Your command failed:
Ocabot commands
More information
|
|
/ocabot merge nobump |
|
On my way to merge this fine PR! |
|
@lmignon The merge process could not be finalized, because command |
This is a forward-port of:
With the applied fixes introduced in: