Review: Needs Fixing

Thanks for your contribution! I think the idea of rendering the wiki in Python 
is very elegant.

Holger, I think putting this script in the document_page module is very 
defendable. Looking at the code, it is clear that the document_page module is 
not only conceptually the replacement of the wiki module, but also a direct 
rewrite of it. This is actually already encoded in the base module's pre script 
that triggers the module renames.

The renaming of the tables and models (ll.343..352) goes together with this 
idea. Small fixes here: I think you do still need to rename the history model. 
You do not need to rename the create_menu table as this is a transient model. 

I do not see why you would make such an effort to precreate and fill columns in 
the pre-script. Typically, this is done in the post-script. If it is to allow 
the orm to set a database constraint 'not null', remember that if you fill them 
properly in the post-script, the orm will set the constraint at the next, 
regular upgrade. Can you say if that was your main concern?

l.364: I am guessing the comment 'Put wiki_wiki content into wiki_groups' 
should be the other way around.

This module aggressively drops deprecated columns. I am generally in favour of 
prefixing them with the generated openupgrade prefix instead, to prevent 
unintentional loss of information. But pending the creation of the ephemeral 
service module to drop deprecated columns in a controlled fashion, I will not 
block this proposal for it.

-- 
https://code.launchpad.net/~savoirfairelinux-openerp/openupgrade-addons/7.0/+merge/190413
Your team Savoir-faire Linux' OpenERP is subscribed to branch 
lp:~savoirfairelinux-openerp/openupgrade-addons/7.0.

-- 
Mailing list: https://launchpad.net/~savoirfairelinux-openerp
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~savoirfairelinux-openerp
More help   : https://help.launchpad.net/ListHelp

Reply via email to