Review: Approve code review, no test
LGTM
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to :
The proposal to merge lp:~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep into
lp:purchase-wkfl has been updated.
Status: Needs review = Approved
For more details, see:
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
--
The proposal to merge lp:~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep into
lp:purchase-wkfl has been updated.
Status: Approved = Merged
For more details, see:
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
--
Review: Approve
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to :
martin, can you change your status here ? Or is it intended to let it in Needs
Fixing ?
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list:
highlights from the similar MP on trunk: Olivier Dony said they will merge on
the core in trunk but not in 7.0, so this extra addon for 7.0 makes sense.
Olivier made some remarks I agree with, they'll be fixed here as well ASAP.
thanks!
--
Review: Needs Fixing
I would suggest that both _key_fields_for_grouping and
_key_fields_for_grouping_lines do a return['a','b']. At the moment I get the
message that the functions return tuples. If they return lists, a simple
res.append('c') is sufficient to add custom fields.
--
Martin,
I am not convinced we need mutability here. Couldn't we still do return res +
('c',)?
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list:
Leonardo, you are right. Both methods will work.
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to
Review: Approve code review, no test
LGTM
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to :
Martin, can you please reconsider your review? your points should be fixed now.
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list:
Review: Abstain
I abstain for the moment until I can review the MP.
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list:
I fixed martin's remarks, and added a new unit test. We're back into needs
review.
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list:
Leonardo Pistone - camptocamp has proposed merging
lp:~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep into lp:purchase-wkfl.
Requested reviews:
martin (riess82)
Romain Deheele - Camptocamp (romaindeheele)
Pedro Manuel Baeza (pedro.baeza)
Related bugs:
Bug #1217869 in OpenERP Addons:
Review: Needs Fixing
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch lp:purchase-wkfl.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to :
I would like to suggest the following changes to purchase_group_hooks.py:
- Change _key_fields_for_grouping_lines to return a list like
_key_fields_for_grouping and not a tuple
- function _update_merged_order_data exists but is not used? This might be the
reason why the addition of field origin
You are right Martin, thanks.
You can set review: Needs fixing and press save comment (no comment is OK
because you already commented).
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch
The proposal to merge lp:~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep into
lp:purchase-wkfl has been updated.
Status: Needs review = Work in progress
For more details, see:
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
--
Review: Abstain
I asked myself about the way to merge origins.
A case seems not managed:
If a PO has origins A B and an other PO has B C, merge result won't be A B
C.
With standard OE processes, it's probably impossible, it's a bit a far-fetched
case, so I abstain me.
--
Your point makes sense, Romain.
I would rather leave it as it is, since:
1. This is just a refactor with a few tests added. It is probably better to
keep the existing functionality for now.
2. I'm not sure that can happen often in practice, and the implementation could
use string manipulation
Pedro, I proposed this to core trunk:
https://code.launchpad.net/~camptocamp/openobject-addons/trunk-refactor-po-merge-lep/+merge/216841
--
https://code.launchpad.net/~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep/+merge/216745
Your team Purchase Core Editors is subscribed to branch
Leonardo Pistone - camptocamp has proposed merging
lp:~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep into lp:purchase-wkfl.
Requested reviews:
Purchase Core Editors (purchase-core-editors)
For more details, see:
Review: Needs Information
Hi, Leonardo, very good workaround, but I think it deserves to propose this
change to 7.0/trunk official branches instead of a module, so that we don't
have to update module with each change/fix made in the replaced part. It
doesn't imply changes in DB layout, so it
Hi Pedro.
For trunk, I am actually going to propose it in the official branches tomorrow.
See lp:~camptocamp/openobject-addons/trunk-refactor-po-merge-lep
For 7.0, I thought it would be easier to do an extra module.
Thanks!
--
24 matches
Mail list logo
