Review: Needs Fixing Hello Lionel,
This looks like a very important addition to the l10n_fr modules, so I'm in favor of merging it :-) Have you perhaps got the chance to ask other French OpenERP integrators to review it? Shouldn't we ask them? A few remarks, nevertheless (line numbers are those of the current diff @r.5755): - l.61: It would be great if the module manifest contained a more complete description, explaining where users can configure its features, and how to use them - *: the source files should presumably bear your own copyright, rather than being copyright "OpenERP SA" ;-) - l.139: assuming that every domain item in the list will be iterable is not correct, what about OR/AND/NOT operators? And actually if you only wanted to have name_search match on the `rib` column in addition to the _rec_name, you could simply override name_search or _name_search, without impacting the normal search(). - l.171: why do you check the constraint again when computing the error message? This will duplicate the check every time... - l.202: this call to search() is actually a normal name_search() call except you're hardcoding the 'name' field. It'd be slightly better to call super.name_search() (which calls name_get for you, so you need a bit of refactoring) Thanks! PS: marking this MP as ``Work In Progress``, when you're ready don't forget to make it ``Needs Review`` again -- https://code.launchpad.net/~numerigraphe/openobject-addons/trunk-rib/+merge/82886 Your team OpenERP Accounting Experts is requested to review the proposed merge of lp:~numerigraphe/openobject-addons/trunk-rib into lp:openobject-addons. _______________________________________________ Mailing list: https://launchpad.net/~openerp-expert-accounting Post to : [email protected] Unsubscribe : https://launchpad.net/~openerp-expert-accounting More help : https://help.launchpad.net/ListHelp
