openerp-community-reviewer team mailing list archive

["Luc De Meyer \(Noviat\)" <luc.demeyer@xxxxxxxxx>]

I updated the https://code.launchpad.net/~luc-demeyer/account-financial-tools/7.0-account_pain branch with the technical changes documented below.

Luc

-----Original Message-----
From: bounces@xxxxxxxxxxxxx [mailto:bounces@xxxxxxxxxxxxx] On Behalf Of Raphaël Valyi - http://www.akretion.com
Sent: woensdag 23 oktober 2013 6:05
To: mp+192256@xxxxxxxxxxxxxxxxxx
Subject: Re: [Merge] lp:~luc-demeyer/account-financial-tools/7.0-account_pain into lp:account-financial-tools

Review: Needs Fixing

Hello Luc,

cool to see Noviat joining the OCA effort. I won't comment functionally but certainly the remarks of Alexis and Pedro are valid.

Technically, there a few caveats that the watchdogs will remind you if I don't, so let's do it:

*  please use orm.Model instead of osv.osv which is deprecated now
*  also use orm.TransientModel instead of osv.memory
*  also you use context={} in methods definitions at lines 309, 324, 327 and 458. You better use the ugly context=None + test against None plumbing instead. Indeed the mutable {} would eventually remain from a method call to another, see http://effbot.org/zone/default-values.htm eventually (Hey but I guess you knew it already).

Regards.


-- 
https://code.launchpad.net/~luc-demeyer/account-financial-tools/7.0-account_pain/+merge/192256
You are the owner of lp:~luc-demeyer/account-financial-tools/7.0-account_pain.


https://code.launchpad.net/~luc-demeyer/account-financial-tools/7.0-account_pain/+merge/192256
Your team OpenERP Community Reviewer is subscribed to branch lp:account-financial-tools.