[RFR] Improve patchwork #3
Conversation
StefanRijnhart
commented
Nov 30, 2020
•
StefanRijnhart
commented
- pinpointed patches needed to run the analysis and removed the rest
- unified upgrade_loading and upgrade_log, taking only methods needed for the analysis
- added some usability to the analysis wizards to use them with odoorpc
| @@ -31,6 +33,10 @@ class UpgradeAnalysis(models.Model): | |||
| ) | |||
|
|
|||
| log = fields.Text(readonly=True) | |||
| upgrade_path = fields.Char( | |||
| default=config.get("upgrade_path", False), | |||
There was a problem hiding this comment.
👍 for making it configurable.
(it is also more visible for the user).
There was a problem hiding this comment.
Yes, as I needed this to set the path from XMLRPC.
| pass | ||
| # </OpenUpgrade> | ||
|
|
||
| PosConfig.post_install_pos_localisation = post_install_pos_localisation |
There was a problem hiding this comment.
do you think this patch is useless ?
There was a problem hiding this comment.
Here again, I could reinstall all Odoo modules with this set of patches, so yes.
| for id, xid, record, info in converted: | ||
| from odoo import api, models | ||
| from ..odoo_patch import OdooPatch | ||
| from ....upgrade_analysis import upgrade_log |
There was a problem hiding this comment.
pythonic question :
I saw you replaced
from ... import upgrade_log
by
from ....upgrade_analysis import upgrade_log
is it a cosmectic change, or is it better for any reason ?
There was a problem hiding this comment.
Oh, I'm just not used to relative imports. Your version works fine and is more direct so I reverted this line.
| method_names = ['_convert_records'] | ||
|
|
||
| @api.model | ||
| def _convert_records(self, records, log=lambda a: None): |
There was a problem hiding this comment.
Huge simplification ! from 179 lines to 21 !
"yeld function" seem easier to patch. Very clean.
| _logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class OdooPatch(object): |
There was a problem hiding this comment.
Very elegant design. Thanks a lot !
Do you think it could be interesting to move this code in a standalone module odoo_patch ?
There was a problem hiding this comment.
Well, the simplistic implementation currently requires that each collection of patches uses their own patch superclass so in the current form it does not make any sense.
|
|
||
| class GenerateWizard(models.TransientModel): | ||
| _name = "upgrade.generate.record.wizard" | ||
| _description = "Upgrade Generate Record Wizard" | ||
|
|
||
| state = fields.Selection([("draft", "Draft"), ("done", "Done")], default="draft") | ||
|
|
||
| # from openupgradelib import openupgrade_tools | ||
| # TODO, SLG, make better a patch in odoo_patch | ||
| # def quirk_standard_calendar_attendances(self): |
There was a problem hiding this comment.
this part is also obsolete in V14 ?
There was a problem hiding this comment.
I could reinstall all Odoo modules with this set of patches, so yes.
StefanRijnhart
force-pushed
the
14.0-ADD-upgrade_analysis
branch
from
da596da to
d6f8927
Compare
|
Thanks for the quick and favorable review! I improved on the relative imports on your suggestion. |
|
thanks ! |
