Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default binder #91

Merged
merged 6 commits into from
Sep 10, 2015
Merged

Default binder #91

merged 6 commits into from
Sep 10, 2015

Conversation

guewen
Copy link
Member

@guewen guewen commented Jul 3, 2015

Follows #76.

@guewen guewen mentioned this pull request Jul 3, 2015
@guewen
Copy link
Member Author

guewen commented Jul 3, 2015

This is the change I want to promote: 1525a4a

As a call to browse() does not hit the database until we first access one of the fields (other than id), I think it is better to always returns a recordset. The API will be easier to use and it reduces the chances of developer errors, we have only one possible type in the return and one knows that she has always to call .id on the result of to_openerp() when she wants the id of a binding. It is also inline with the new API which returns recordsets instead of ids in search().

@lmignon
Copy link
Contributor

lmignon commented Jul 3, 2015

👍 (code review)

@guewen guewen force-pushed the anybox-default_binder branch from 1525a4a to 0dfd98a Compare July 3, 2015 13:13
@guewen guewen force-pushed the anybox-default_binder branch from 7ddadf9 to 4837500 Compare July 14, 2015 14:30
@guewen guewen force-pushed the anybox-default_binder branch from 4837500 to 320e779 Compare September 10, 2015 12:14
@guewen
Copy link
Member Author

guewen commented Sep 10, 2015

Rebased. I would ❤️ to get this PR merged.

@sebastienbeau
Copy link
Member

👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍
Yeeesssss, I totaly approve this PR

@bguillot
Copy link
Contributor

of course 👍 !
I missed this one I approved the previous one ...

sebastienbeau added a commit that referenced this pull request Sep 10, 2015
@sebastienbeau sebastienbeau merged commit 1f24696 into OCA:8.0 Sep 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants