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

Port hr_employee_benefit to 8.0 #137

Merged
merged 10 commits into from
Oct 2, 2015

Conversation

ghost
Copy link

@ghost ghost commented Sep 14, 2015

No description provided.

@ghost
Copy link
Author

ghost commented Sep 19, 2015

@max3903 @mmalorni could you test this module please ?

@max3903 max3903 added this to the 8.0 milestone Sep 19, 2015
@max3903 max3903 self-assigned this Sep 19, 2015
@max3903 max3903 mentioned this pull request Oct 1, 2015
58 tasks
In case of trouble, please check there if your issue has already been reported.
If you spotted it first, help us smashing it by providing a detailed and welcomed feedback
`here <https://github.com/OCA/{project_repo}/issues/new?body=module:%20{module_name}%0Aversion:%20{version}%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.

Copy link

Choose a reason for hiding this comment

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

You let the {project_repo} {module_name}, ...

@Ehtaga
Copy link

Ehtaga commented Oct 1, 2015

Code review: 👍

@ghost ghost force-pushed the 8.0-hr_employee_benefit branch from 0fadb96 to 2ae9c0a Compare October 1, 2015 22:38
return res

@api.multi
def _filter_benefits(self, payslip, **kwargs):
Copy link

Choose a reason for hiding this comment

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

@dufresnedavid Why do we need **kwargs here?

Copy link

Choose a reason for hiding this comment

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

@dufresnedavid If we are returning a type other than self maybe it would be the case to declare the decaorator @api.returns().

Copy link
Author

Choose a reason for hiding this comment

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

@joaoalf we need kwargs to enable other modules to inherit the function easily.

from .hr_employee_benefit_rate import get_amount_types


class HrEmployeeBenefit(models.Model):
Copy link

Choose a reason for hiding this comment

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

@dufresnedavid Just a sugestion:

class HrEmployeeBenefit(models.Model):
    """Employee Benefit"""

    _name = 'hr.employee.benefit'
    _description = _(__doc__)

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@coleste
Copy link

coleste commented Oct 2, 2015

👍

@ghost ghost force-pushed the 8.0-hr_employee_benefit branch from 1d60067 to ac778a3 Compare October 2, 2015 00:23
@ghost ghost force-pushed the 8.0-hr_employee_benefit branch from ac778a3 to 589ade7 Compare October 2, 2015 00:33
@joaoalf
Copy link

joaoalf commented Oct 2, 2015

👍

max3903 pushed a commit that referenced this pull request Oct 2, 2015
@max3903 max3903 merged commit fcbaa1d into OCA:8.0 Oct 2, 2015
@max3903 max3903 deleted the 8.0-hr_employee_benefit branch October 2, 2015 11:53
sambarros pushed a commit to sambarros/hr that referenced this pull request Jul 26, 2018
[BSSFL-207] Add product responsibles and product followers
Mraimou pushed a commit to camptocamp/hr that referenced this pull request Nov 25, 2019
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

5 participants