← Back to team overview

credativ team mailing list archive

Re: lp:~sylvain-legal/openupgrade-server/three-functions-for-addons-migration into lp:openupgrade-server

 

Review: Needs Fixing

Again, the question of having copyright assigned to George Arbitbol pops up.

warn_possible_dataloss: Can you tell me the exact use case for this function? I noticed that in 7.0, a number of functionalies have been split off to separate 'glue' modules. For example, there is now sale_stock for functionality that was first in the sale and stock modules. However, this module will be installed automatically if sale and stock are both installed, which will be the case if you have the sale module installed in 6.1 as it still depended on stock.

The other methods seem very useful to me. I have three minor comments for them.

l.58: I think mentioning the year is useful for copyright assignments. 

    # This module copyright (C) 2013 Georges Abitbol

l.87: you are not supposed to perform variable substitution inline when using cr.execute() (unless you are substituting table names as there is no other way). The following should work:

WHERE id=%s""",
(partner_address_id,))

Same for l. 99.

-- 
https://code.launchpad.net/~sylvain-legal/openupgrade-server/three-functions-for-addons-migration/+merge/174668
Your team OpenUpgrade Committers is subscribed to branch lp:openupgrade-server.


References