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

[12.0][MIG] res_config_settings_enterprise_remove #7

Closed
wants to merge 8 commits into from
Closed

[12.0][MIG] res_config_settings_enterprise_remove #7

wants to merge 8 commits into from

Conversation

astirpe
Copy link
Member

@astirpe astirpe commented Oct 11, 2018

#6

@astirpe astirpe mentioned this pull request Oct 11, 2018
2 tasks
@eLBati
Copy link
Member

eLBati commented Oct 21, 2018

Hi @astirpe , could you also add this a017bc5 ?

@pedrobaeza
Copy link
Member

The problem with <delete> records is that they give a warning when the record is not found, so updating 2 times the module results in an ugly log, but right now I can't think in a better solution.

@astirpe
Copy link
Member Author

astirpe commented Oct 21, 2018

@eLBati @pedrobaeza as stated in the description on the readme, this module is not meant to delete the enterprise modules from the modules list. It's only meant to remove them from the config settings view. I would rather create a separate module for that. Or discuss the possibility to modify this module, but in a separate PR. I honestly don't like to introduce warnings in the log when doing an update all. So if it's OK for you, I would keep this PR as it is: a pure porting of the module.

@pedrobaeza
Copy link
Member

OK, let's keep this PR as is, but introduce later that.

@eLBati
Copy link
Member

eLBati commented Oct 22, 2018

Ok

Brett Wood and others added 8 commits October 30, 2018 16:23
…o reduce loops, complexity, and eliminate any risky xpath queries.
* [MIG][10.0] Migrate 'res_config_settings_enterprise_remove'

* [FIX] Use inherit to override 'res.config.settings'.

* [FIX] Remove upgrade_radio widget and group title

* [FIX] Improviments in code to remove upgrade fields

* [NEW] Remove enterprise upgrade from 'project config settings'

* [FIX] Add xpath to remove upgrade fields from 'General Settings'
@levkar
Copy link

levkar commented Nov 25, 2018

I made a simple 12.0 migration PR #11 before i noticed this PR.

Here is the way to avoid warning messages:

<delete model="ir.module.module" search="[('to_buy', '=', True)]"/>

But the name of the module ("res_config_settings_enterprise_remove") can be misleading. Maybe it should be another module named "module_enterprise_remove" ? Or a new module named "enterprise_remove" can be introduced to include both functionalities. I can cancel my PR and/or help with a new PR. Let me know what you think.

One last question, why don't we have modules in this repo with auto_install=True?

@astirpe
Copy link
Member Author

astirpe commented Nov 30, 2018

@levkar thank you for the tip! I agree with you, an extra module is needed. Or an alternative "enterprise_remove" module would be great, but this solution will prevent the user to install enterprise modules! Both proposals are fine to me.
Would you like to go ahead and open a PR? Let me know...

@levkar
Copy link

levkar commented Dec 5, 2018

@astirpe I created a new pull request #12.

@astirpe
Copy link
Member Author

astirpe commented Dec 10, 2018

Closing in favour of #12

@astirpe astirpe closed this Dec 10, 2018
@astirpe astirpe deleted the 12_mig_res_config_settings_enterprise_remove branch January 3, 2019 11:09
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.

6 participants