[l10n_ca_hr_payroll] Migration to version 8.0 #92
Conversation
|
Hey @rigo1985, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
|
@rigo1985 do you know that you can not use deduction tables in an electronic payroll system in Canada? You must implement the formulas. The deduction tables are only for people who calculate their payroll manually (in Excel for example). |
| @@ -1,307 +1,272 @@ | |||
| # -*- coding:utf-8 -*- | |||
| ############################################################################## | |||
There was a problem hiding this comment.
@rigo1985 Hi, thanks for your contribution.
In here why you remove completely the copyright notice? Most OCA modules user the shorter format for the copyright notice but we don't remove it.
| else: | ||
| res['name'] += ' / Federal' | ||
| @api.onchange('year') | ||
| def onchange_year(self): |
There was a problem hiding this comment.
@rigo1985 You should start the on onchange method name with _.
| weeks_of_vacation = fields.Integer(string='Number of weeks of vacation', | ||
| required=True, default=2) | ||
|
|
||
| #END |
There was a problem hiding this comment.
@rigo1985 You need to finish the file with a \n. Also, I'm not very found of the #END comment at the end (it's a meaningless information) but it's just my personnal taste so it's not blocking.
|
@rigo1985 Please, don't forget to sign the CLA. |
|
@rigo1985 Can you add some unittests please? |
|
@rigo1985 Please check also the travis result, there are some PEP8 errors in the module. |
[l10n_ca_hr_payroll] Migration to version 8.0