The proposal to merge lp:~therp-nl/ocb-addons/lp754339 into lp:ocb-addons has
been updated.
Status: Needs review => Rejected
For more details, see:
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
--
Your team OpenERP Community Backports is subscribed to branch lp:ocb-
Holger Brunn (Therp) has proposed merging lp:~therp-nl/ocb-addons/lp754339 into
lp:ocb-addons.
Requested reviews:
OpenERP Community Backports Team (ocb)
Related bugs:
Bug #754339 in OpenERP Community Backports (Addons): "Partner assigned Price
List is not applied with the creation of a custo
The proposal to merge lp:~therp-nl/ocb-addons/lp754339 into lp:ocb-addons has
been updated.
Status: Needs review => Work in progress
For more details, see:
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+mer
Holger Brunn (Therp) has proposed merging lp:~therp-nl/ocb-addons/lp754339 into
lp:ocb-addons.
Requested reviews:
Stefan Rijnhart (Therp) (stefan-therp)
Holger Brunn (Therp) (hbrunn)
Related bugs:
Bug #754339 in OpenERP Community Backports (Addons): "Partner assigned Price
List is not appl
The proposal to merge lp:~therp-nl/ocb-addons/lp754339 into lp:ocb-addons has
been updated.
Commit Message changed to:
[FIX] This is a port of the fix to have prices in manually added invoice lines
originate from the partner's pricelist, if any. If there are no pricelists
involved, nothing sho
The proposal to merge lp:~therp-nl/ocb-addons/lp754339 into lp:ocb-addons has
been updated.
Status: Needs review => Approved
For more details, see:
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/18910
The proposal to merge lp:~therp-nl/ocb-addons/lp754339 into lp:ocb-addons has
been updated.
Status: Approved => Merged
For more details, see:
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your
The proposal to merge lp:~therp-nl/ocb-addons/lp754339 into lp:ocb-addons has
been updated.
Status: Merged => Needs review
For more details, see:
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Review: Disapprove
Development for 7.0 has moved to github on https://github.com/OCA/ocb - please
move your merge proposal there if it is still valid.
(I close and reject this in order to have a cleaner overview for 6.1 MPs which
indeed have to be done on launchpad)
--
https://code.launchpad.n
Review: Needs Fixing
Thanks for taking the effort of forward porting (and improving) this old fix
for 6.1 to 7.0! I sure hope this important fix lands in upstream one day.
l.66 seems wrong. I think in 7.0 you need to append it to the invoice line's
name field like the original code does in l.42
Thanks also from my side!
When I compare this merge proposal with the original one for 6.1 therp
backports (which became ocb 6.1), I note a few things.
(compare:
https://code.launchpad.net/~therp-nl/therp-backports/addons-6.1-lp754339_pricelist_on_invoice_line/+merge/157829)
1. There is no lay
This branch is a (nearly) verbatim copy of the old trunk branch
https://code.launchpad.net/~therp-nl/openobject-addons/ronald@therp.nl_fix_invoice_pricelist_lp754339/+merge/142021
Ronald: Any chance you remember why you introduced the changes you mention?
The test sounds like a very good idea, se
Holger: I did not, at least not knowingly, introduce those changes.
I think they can be undone, as they seem to have nothing to do with the problem
that this merge is supposed to fix.
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports
Review: Resubmit
Thank you two for your input, I removed the offending changes (still can't
really explain where they come from), reverted the cosmetic one and added a test
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is sub
Review: Approve
Thanks, great work! I think you can save some lines by calling
product_product._compute_price() but I am guessing that either Ronald or you
didn't think it was a good idea to call a private method which is fair enough.
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339
Forgot to mention: product_product._compute_price calculates a given price
between two uoms which could replace ll. 107 to 114
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Mailing
Review: Resubmit
good idea, thanks!
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to : openerp-community-rev
Review: Approve
Very nice. Thank you.
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to : openerp-community-r
Thank you!
I can migrate from gnucash multiple banks for multiple clients now! This works
in ocb-addons, had trouble getting unit price to change with your other patch
for openobject-addons
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Ba
Thanks for testing this! Could you (as a comment on the merge proposal for
openobject-addons) elaborate on the problems you faced there?
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Review: Approve
Approve. Tested the original code. Present code LGTM.
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
P
Review: Approve code review, no test
Hi
It's OK for me (already fix in 6.1)
Regards,
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Mailing list: https://launchpad.net/~openerp-com
Hi
After the merge, the buildbit failed, the problem is in the line 106-107,
method called since the browse record have too enougth arguments
(_compute_price() takes at most 6 arguments (10 given))
I must revert proprely this merge proposal
Regards,
2013-11-02 10:33:14,060 25494 ERROR openerp
Review: Needs Fixing
I have revert this MP
This MP must be fix, see the previous comment.
Regards,
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Mailing list: https://launchpad.
Review: Needs Fixing
Christophe, thank you for your swift actions! I see that _compute_price is
called on a uom browse record instead of on a model object, which implicitely
passes cr, uid, [res_id] and context. The first three arguments are passed
explicitely too in line 106, while the method
Note that the changes must be reapplied manually (i.e. using diff/patch or so)
on this branch after a merge of the target branch, because of the revert
(currently, merging this branch on a copy of ocb-addons/7.0 will tell you
'nothing to do')
--
https://code.launchpad.net/~therp-nl/ocb-addons/
Review: Resubmit
Thanks for your efforts!
I think I found another big problem: Choosing list_price and standard_price
were swapped in 63ff.
Let's tightly review this again before merging.
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Back
Review: Needs Fixing test, code review
Improvements look good. Tests run fine now. I hope we can get this back in
again. How can your average OpenERP partner sell service hours without this
patch?
There is a trivial conflict now in an __init__.py.
Looks like the proposal to upstream needs to b
Thanks, I did the merge and also updated trunk's patch
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to : ope
Review: Approve
Thanks!
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to : openerp-community-reviewer@lists.
Review: Disapprove should be a module
This looks fine, but I feel it should really be a module rather than a patch.
I would certainly not think of OCB if I needed this feature, and those using
the official addons could use it too.
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merg
In principle, I agree with Lionel.
Two arguments for putting this into ocb rather than into a module
- doing this in a module most likely won't work well with other modules that
fiddle with the onchange method (weak argument)
- it's in 6.1ocb, so an upgrade to 7.0ocb would break existing functi
Review: Abstain
Thanks for making this clear, I see your point.
I'll let the OCA team decide this on a strategic level.
--
https://code.launchpad.net/~therp-nl/ocb-addons/lp754339/+merge/189107
Your team OpenERP Community Backports Team is subscribed to branch
lp:ocb-addons.
--
Mailing list: h
33 matches
Mail list logo
