Merge lp://staging/~camptocamp/ocb-addons/ocb-7.0-fix_1302630_document_search_order_by-rde into lp://staging/ocb-addons

Proposed by Alexandre Fayolle - camptocamp
Status: Rejected
Rejected by: Holger Brunn (Therp)
Proposed branch: lp://staging/~camptocamp/ocb-addons/ocb-7.0-fix_1302630_document_search_order_by-rde
Merge into: lp://staging/ocb-addons
Diff against target: 25 lines (+7/-1)
1 file modified
document/document.py (+7/-1)
To merge this branch: bzr merge lp://staging/~camptocamp/ocb-addons/ocb-7.0-fix_1302630_document_search_order_by-rde
Reviewer Review Type Date Requested Status
Holger Brunn (Therp) Disapprove
Yannick Vaucher @ Camptocamp code review, no test Approve
Alexandre Fayolle - camptocamp code review, no test Approve
Lionel Sausin - Initiatives/Numérigraphe (community) Needs Fixing
Leonardo Pistone code review Approve
Review via email: mp+214486@code.staging.launchpad.net

Description of the change

To post a comment you must log in.
Revision history for this message
Leonardo Pistone (lepistone) wrote :
review: Approve (code review)
Revision history for this message
Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives) wrote :

- "id" is not a great variable name, python has id() already.
- wouldn't it be the same to write "result = list(ids)" at line 21?

review: Needs Fixing
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

> - wouldn't it be the same to write "result = list(ids)" at line 21?

`list(ids)` wouldn't keep the order of ids.

Revision history for this message
Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives) wrote :

I see, line 21 it deserves a comment then.

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

LGTM

review: Approve (code review, no test)
Revision history for this message
Holger Brunn (Therp) (hbrunn) :
review: Approve (code review)
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

LGTM

review: Approve (code review, no test)
Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

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)

review: Disapprove

Unmerged revisions

10077. By Romain Deheele - Camptocamp

[FIX] document: search method overload should take into account order by clause

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.