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

[IMP] Added reference and name_get for Business Requirement #55

Merged
merged 3 commits into from
Feb 14, 2017
Merged

[IMP] Added reference and name_get for Business Requirement #55

merged 3 commits into from
Feb 14, 2017

Conversation

astirpe
Copy link
Member

@astirpe astirpe commented Oct 26, 2016

Fixes #45 and #37

@rafaelbn rafaelbn added this to the 8.0 milestone Nov 10, 2016
@@ -15,6 +15,7 @@ def test_get_level(self):
br_vals1 = {
'name': ' test',
'description': 'test',
'ref': 'Ref1',
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usual we use / for the default value in reference fields.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

Copy link
Sponsor Member

@dreispt dreispt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@astirpe
Copy link
Member Author

astirpe commented Dec 30, 2016

@elicoidal May I ask you to take a look at this PR?

@elicoidal
Copy link
Contributor

slight change on the void reference case. Other than that 👍

@astirpe
Copy link
Member Author

astirpe commented Feb 14, 2017

Rebased branch

@elicoidal
Copy link
Contributor

Good to go.
Thanks!

@elicoidal elicoidal merged commit 84197ed into OCA:8.0 Feb 14, 2017
@astirpe astirpe deleted the 80_fix_issue45 branch February 14, 2017 09:38
@astirpe
Copy link
Member Author

astirpe commented Feb 14, 2017

Thank you!
We need to remember to port these changes to V9 also.

@elicoidal
Copy link
Contributor

Yes indeed...
How should we control that? create a WIP PR for the moment?

@astirpe
Copy link
Member Author

astirpe commented Feb 14, 2017

PR for porting the changes to V9, here it is: #68

This was referenced Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants