← Back to team overview

credativ team mailing list archive

Re: [Merge] lp:~savoirfairelinux-openerp/openupgrade-addons/7.0 into lp:openupgrade-addons

 

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 OpenUpgrade Committers is subscribed to branch lp:openupgrade-addons.


References