Re: [Openerp-community] [Merge] lp:~openerp-community/server-env-tools/6.1-mass_editing into lp:server-env-tools/6.1

2014-03-17 Thread Sandy Carter (http://www.savoirfairelinux.com)
Review: Needs Information @Stefan, could you please explain the dataloss issue in this frame, I don't quite understand. How does the branch you propose come into play here? -- https://code.launchpad.net/~openerp-community/server-env-tools/6.1-mass_editing/+merge/161619 Your team OpenERP Communit

Re: [Openerp-community] [Merge] lp:~therp-nl/server-env-tools/6.1-mass_editing-fix_dataloss into lp:~openerp-community/server-env-tools/6.1-mass_editing

2014-03-18 Thread Sandy Carter (http://www.savoirfairelinux.com)
Review: Needs Fixing code review @Stefan: I thin you missed the point of vals[model_field]: There is a chance of a KeyError if model_field not in val and a chance of index error if len(vals[model_field]) < 1 or len(vals[model_field][0]) < 3 -- https://code.launchpad.net/~therp-nl/server-env-tool

Re: [Openerp-community] [Merge] lp:~therp-nl/server-env-tools/6.1-mass_editing-fix_dataloss into lp:~openerp-community/server-env-tools/6.1-mass_editing

2014-03-18 Thread Sandy Carter (http://www.savoirfairelinux.com)
Review: Approve code review, no test OK, sorry about the misunderstanding. Nevertheless, I would prefer to see some assertions, gets and tries as this function looks likely to throw an IndexError. But I see you've take them into account (removal of redundant checks, values checking, etc) -- htt

Re: [Openerp-community] [Merge] lp:~openerp-community/server-env-tools/6.1-mass_editing into lp:server-env-tools/6.1

2014-03-19 Thread Sandy Carter (http://www.savoirfairelinux.com)
Review: Approve -- https://code.launchpad.net/~openerp-community/server-env-tools/6.1-mass_editing/+merge/161619 Your team OpenERP Community is subscribed to branch lp:~openerp-community/server-env-tools/6.1-mass_editing. ___ Mailing list: https://la

[Openerp-community] [Merge] lp:~openerp-community/server-env-tools/6.1-mass_editing into lp:server-env-tools/6.1

2014-03-20 Thread Sandy Carter (http://www.savoirfairelinux.com)
The proposal to merge lp:~openerp-community/server-env-tools/6.1-mass_editing into lp:server-env-tools/6.1 has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~openerp-community/server-env-tools/6.1-mass_editing/+merge/161619 -- https://code.

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-canada/7.0-l10n_ca_account_check_writing into lp:openerp-canada

2014-04-30 Thread Sandy Carter (http://www.savoirfairelinux.com)
Review: Approve LGTM, thanks The future module is worth looking at for future.str_as_unicode https://pypi.python.org/pypi/future/0.3.1 -- https://code.launchpad.net/~openerp-community/openerp-canada/7.0-l10n_ca_account_check_writing/+merge/216708 Your team OpenERP Community is subscribed to bra

[Openerp-community] [Merge] lp:~openerp-community/openerp-canada/7.0-l10n_ca_account_check_writing into lp:openerp-canada

2014-05-06 Thread Sandy Carter (http://www.savoirfairelinux.com)
The proposal to merge lp:~openerp-community/openerp-canada/7.0-l10n_ca_account_check_writing into lp:openerp-canada has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~openerp-community/openerp-canada/7.0-l10n_ca_account_check_writing/+merge

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-canada/7.0-missing-payroll-access into lp:openerp-canada

2014-06-11 Thread Sandy Carter (http://www.savoirfairelinux.com)
Review: Needs Fixing Diff comments: > === modified file 'l10n_ca_hr_payroll/__openerp__.py' > --- l10n_ca_hr_payroll/__openerp__.py 2013-02-27 20:02:47 + > +++ l10n_ca_hr_payroll/__openerp__.py 2014-06-11 15:00:16 + > @@ -38,6 +38,7 @@ > 'l10n_ca_toponyms', > ], > 'da

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-canada/7.0-missing-payroll-access into lp:openerp-canada

2014-06-30 Thread Sandy Carter (http://www.savoirfairelinux.com)
Review: Approve LGTM -- https://code.launchpad.net/~openerp-community/openerp-canada/7.0-missing-payroll-access/+merge/225031 Your team OpenERP Community is subscribed to branch lp:~openerp-community/openerp-canada/7.0-missing-payroll-access. ___ Mail

Re: [Openerp-community] [Merge] lp:~openerp-community/openerp-canada/7.0-missing-payroll-access into lp:openerp-canada

2014-07-18 Thread Sandy Carter (http://www.savoirfairelinux.com)
Review: Resubmit Please move this MP to https://github.com/OCA/l10n-canada More info at https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub -- https://code.launchpad.net/~openerp-community/openerp-canada/7.0-missing-payroll-access/+merge/226187 Your team OpenER

[Openerp-community] [Merge] lp:~openerp-community/openerp-canada/7.0-missing-payroll-access into lp:openerp-canada

2014-07-18 Thread Sandy Carter (http://www.savoirfairelinux.com)
The proposal to merge lp:~openerp-community/openerp-canada/7.0-missing-payroll-access into lp:openerp-canada has been updated. Status: Needs review => Rejected For more details, see: https://code.launchpad.net/~openerp-community/openerp-canada/7.0-missing-payroll-access/+merge/226187 -- ht