Merge lp://qastaging/~camptocamp/ocb-addons/7.0-fix-1189480 into lp://qastaging/ocb-addons

Proposed by Nicolas Bessi - Camptocamp
Status: Rejected
Rejected by: Yannick Vaucher @ Camptocamp
Proposed branch: lp://qastaging/~camptocamp/ocb-addons/7.0-fix-1189480
Merge into: lp://qastaging/ocb-addons
Diff against target: 35 lines (+12/-6)
1 file modified
project/project.py (+12/-6)
To merge this branch: bzr merge lp://qastaging/~camptocamp/ocb-addons/7.0-fix-1189480
Reviewer Review Type Date Requested Status
Yannick Vaucher @ Camptocamp superseeded Disapprove
Matthieu Dietrich @ camptocamp (community) Disapprove
Holger Brunn (Therp) code review Approve
Review via email: mp+168463@code.qastaging.launchpad.net

Description of the change

Quick fix of bug 1189480

To post a comment you must log in.
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Can some one review this please?

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

would be great to add a yaml test for this one.

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Yes but I won't be able do do it before july

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

ok not before september...

Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

I agree with your code but also agree with the need for a test. Setting this to work in progress meanwhile.

review: Approve (code review)
Revision history for this message
Matthieu Dietrich @ camptocamp (mdietrich-c2c) wrote :

Actually, I investigated this branc, since it caused performance issues in our instance using it. The multiple read() are a major impact, given it will be done for each child project.
It seems that the computation is off a bit as well; projects with children will not add their own values for the fields, but those values will be added to their parents.

I'll rework this and do a YAML test to validate it.

review: Needs Fixing
Revision history for this message
Matthieu Dietrich @ camptocamp (mdietrich-c2c) wrote :
review: Disapprove
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) :
review: Disapprove (superseeded)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.