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

[MIG] hr_family: Migration to v9 #268

Merged
merged 7 commits into from
Oct 21, 2016
Merged

Conversation

damdam-s
Copy link
Member

No description provided.

@damdam-s
Copy link
Member Author

@gurneyalex ping
#157

@pedrobaeza pedrobaeza changed the title Migration to v9 [MIG] hr_family: Migration to v9 Oct 20, 2016
@pedrobaeza pedrobaeza mentioned this pull request Oct 20, 2016
65 tasks
@leemannd
Copy link

👍

Copy link

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

👍 Overall it's fine. I'd change <openerp><data> with <odoo>, that's it.

@damdam-s
Copy link
Member Author

thanks @sylvain-garancher for the tip on translations

@Garamotte
Copy link

The standard hr.employee model has three values for the gender field : Male, Female, or Other.
Maybe you should add Other for hr.employee.children accordingly.

Otherwise 👍

Copy link
Member

@andhit-r andhit-r left a comment

Choose a reason for hiding this comment

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

@damdam-s thanks for your contributions

Functionality review: Ok (tested on runbot)
Code: Some remarks (see inline comment) and please address #268 (review)

@@ -16,26 +18,40 @@ For further information, please visit:

Copy link
Member

Choose a reason for hiding this comment

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

Remove

[FIX] The standard hr.employee model has three values for the gender
field : Male, Female, or Other. Use them for children object
@damdam-s
Copy link
Member Author

@simahawk done
@sylvain-garancher done
@andhit-r I don't see what you want me to remove. README.rst file has been updated according OCA guidelines

@andhit-r
Copy link
Member

Its https://github.com/OCA/hr/pull/268/files#diff-f495b6dc5b6053b20131ef11b2ca2a5dR17. The for further information ..., you dont need that part. But thats minor. So 👍

@damdam-s
Copy link
Member Author

@andhit-r ok. let's remove it. thanks for explaination

@damdam-s
Copy link
Member Author

@tafaRU @dreispt @dufresnedavid @jgrandguillaume @eLBati @feketemihai as PSC member of this project, can one of you review this or (better) merge it ?
Thanks in advance

@feketemihai
Copy link
Member

👍

@feketemihai feketemihai merged commit a983cd0 into OCA:9.0 Oct 21, 2016
@damdam-s
Copy link
Member Author

@feketemihai thank you !

sambarros pushed a commit to sambarros/hr that referenced this pull request Jul 26, 2018
BIZ-930 Changed behavior of sale order invoice status
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.

None yet

7 participants