Merge lp://staging/~savoirfairelinux-openerp/openerp-hr/language into lp://staging/openerp-hr/6.1

Status: Rejected
Rejected by: Sandy Carter (http://www.savoirfairelinux.com)
Proposed branch: lp://staging/~savoirfairelinux-openerp/openerp-hr/language
Merge into: lp://staging/openerp-hr/6.1
Diff against target: 687 lines (+655/-0)
6 files modified
hr_language/__init__.py (+24/-0)
hr_language/__openerp__.py (+51/-0)
hr_language/hr_language.py (+49/-0)
hr_language/hr_language_view.xml (+67/-0)
hr_language/i18n/hr_language.pot (+462/-0)
hr_language/security/ir.model.access.csv (+2/-0)
To merge this branch: bzr merge lp://staging/~savoirfairelinux-openerp/openerp-hr/language
Reviewer Review Type Date Requested Status
Alexandre Fayolle - camptocamp code review, no tests Approve
Yannick Vaucher @ Camptocamp Needs Information
Pedro Manuel Baeza Needs Information
Joao Alfredo Gama Batista code review. no test Needs Fixing
Maxime Chambreuil (http://www.savoirfairelinux.com) code review Approve
Review via email: mp+194925@code.staging.launchpad.net

Description of the change

[ADD] add hr_language module.It adds a new menu in hr module.This module depends on hr module

To post a comment you must log in.
Revision history for this message
Maxime Chambreuil (http://www.savoirfairelinux.com) (max3903) :
review: Approve (code review)
307. By EL HADJI DEM <email address hidden>

[IMP] add pot file

Revision history for this message
Joao Alfredo Gama Batista (joao-gama) wrote :

Hi El Hadji,

Thanks again for your contribution. I just did a review for your other merge proposal (https://code.launchpad.net/~savoirfairelinux-openerp/openerp-hr/experience/+merge/194926) and the same points apply.

l.98.101.117.120.125

review: Needs Fixing (code review. no test)
308. By El Hadji Dem (http://www.savoirfairelinux.com)

[PEP8] pep8

309. By El Hadji Dem (http://www.savoirfairelinux.com)

[pep8] pep8

310. By Maxime Chambreuil (http://www.savoirfairelinux.com)

[FIX] Based on MP reviews. Improve XML readability

Revision history for this message
El Hadji Dem (http://www.savoirfairelinux.com) (eh-dem) wrote :

thanks maxime

311. By Maxime Chambreuil (http://www.savoirfairelinux.com)

[IMP] Update translation file

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Hi, El Hadji,

Thanks for the MP. I see something strange with language selection, because all the language names needs to be translated and appear in the pot file. Isn't any other machanism that uses translations made on base module.

Regards.

review: Needs Information
Revision history for this message
Maxime Chambreuil (http://www.savoirfairelinux.com) (max3903) wrote :

Hello Pedro,

Good catch Pedro ;) Problem is that an employee may speak, read or write a language which is not installed in OpenERP. That's why we decided to get the list of languages from tools.scan_languages().

Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

Wouldn't it be more interesting to have a level of language like http://en.wikipedia.org/wiki/Common_European_Framework_of_Reference_for_Languages
Or something like Linkedin language levels ?
Than a simple true / false ?

review: Needs Information
Revision history for this message
Maxime Chambreuil (http://www.savoirfairelinux.com) (max3903) wrote :

@Yannick

You are right. Same thing would apply for skills evaluation.

We wanted to start simple to generate a resume, where languages and skills would appear or not.

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

LGTM

review: Approve (code review, no tests)
Revision history for this message
Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter) wrote :

Unmerged revisions

311. By Maxime Chambreuil (http://www.savoirfairelinux.com)

[IMP] Update translation file

310. By Maxime Chambreuil (http://www.savoirfairelinux.com)

[FIX] Based on MP reviews. Improve XML readability

309. By El Hadji Dem (http://www.savoirfairelinux.com)

[pep8] pep8

308. By El Hadji Dem (http://www.savoirfairelinux.com)

[PEP8] pep8

307. By EL HADJI DEM <email address hidden>

[IMP] add pot file

306. By EL HADJI DEM <email address hidden>

[add] add hr_language module

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches