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

[12.0][MIG] hr_employee_language #783

Merged
merged 54 commits into from Mar 24, 2020

Conversation

Saran440
Copy link
Member

@Saran440 Saran440 commented Jan 27, 2020

  • Change module name hr_language -> hr_employee_language because this module related to employee.
  • Add search filters in hr.employee.language
  • [IMP] Add boolean can_listen in hr.employee.language
  • [IMP] Change menu position to root

and How can I manage file l10n in old name hr_language?

@lfreeke
Copy link

lfreeke commented Feb 21, 2020

Hi @Saran440 I wonder why the description field of the Language is required? Doesn't feel really user friendly. What is the reason that the field is required?

@Saran440 Saran440 force-pushed the 12.0-mig-hr_employee_language branch from af2fe31 to f4f03e9 Compare February 24, 2020 10:47
@Saran440
Copy link
Member Author

Hi @lfreeke,
This field is required from older version and I'm not sure why.
But It's make sense to not required.
Thank you for your advice. :)

Copy link

@lfreeke lfreeke left a comment

Choose a reason for hiding this comment

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

Functional test 👍

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

Copy link

@jarroyomorales jarroyomorales left a comment

Choose a reason for hiding this comment

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

Functional review and LGTM in general. The only thing I dont like is putting all the lines of hr.employee.language in the configuration menu. The configuration menu is usually used to set contract types, job positions, tags.. things like that and I think having this menu there is not very intuitive.
But since putting the menu in the hr_root may be too much maybe there is no better option that putting it there...

@Saran440 Saran440 force-pushed the 12.0-mig-hr_employee_language branch 3 times, most recently from fad0ced to c640b93 Compare March 9, 2020 02:31
@Saran440 Saran440 closed this Mar 9, 2020
@Saran440 Saran440 force-pushed the 12.0-mig-hr_employee_language branch from c640b93 to bed8c66 Compare March 9, 2020 02:35
@Saran440 Saran440 reopened this Mar 9, 2020
acsonefho and others added 14 commits March 9, 2020 09:40
Currently translated at 31.9% (30 of 94 strings)

Translation: hr-10.0/hr-10.0-hr_language
Translate-URL: https://translation.odoo-community.org/projects/hr-10-0/hr-10-0-hr_language/fr/
Updated by Update PO files to match POT (msgmerge) hook in Weblate.
Currently translated at 91.5% (86 of 94 strings)

Translation: hr-10.0/hr-10.0-hr_language
Translate-URL: https://translation.odoo-community.org/projects/hr-10-0/hr-10-0-hr_language/de/
Currently translated at 97.9% (92 of 94 strings)

Translation: hr-10.0/hr-10.0-hr_language
Translate-URL: https://translation.odoo-community.org/projects/hr-10-0/hr-10-0-hr_language/de/
Currently translated at 100.0% (94 of 94 strings)

Translation: hr-10.0/hr-10.0-hr_language
Translate-URL: https://translation.odoo-community.org/projects/hr-10-0/hr-10-0-hr_language/de/
Currently translated at 100.0% (94 of 94 strings)

Translation: hr-10.0/hr-10.0-hr_language
Translate-URL: https://translation.odoo-community.org/projects/hr-10-0/hr-10-0-hr_language/es/
@Saran440 Saran440 force-pushed the 12.0-mig-hr_employee_language branch 3 times, most recently from dabd05a to 21c6050 Compare March 9, 2020 02:52
@Saran440
Copy link
Member Author

Saran440 commented Mar 9, 2020

@jarroyomorales That sound great idea. I change position to root menu.
Can you check again, please.

@jarroyomorales
Copy link

Nice! Thank you! Can some commits get squashed?

@Saran440 Saran440 force-pushed the 12.0-mig-hr_employee_language branch from 21c6050 to d5f5ee0 Compare March 10, 2020 02:31
@Saran440
Copy link
Member Author

@jarroyomorales merged commit to 2 PR. migration and implement for cherry-pick to v13 #784 or backport to other version. Thank you for your advice :)

@Saran440
Copy link
Member Author

/ocabot merge

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 12.0-ocabot-merge-pr-783-by-Saran440-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Mar 24, 2020
Signed-off-by Saran440
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 12.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 12.0-ocabot-merge-pr-783-by-Saran440-bump-no, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit bc531e7 into OCA:12.0 Mar 24, 2020
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at cfc3bd8. Thanks a lot for contributing to OCA. ❤️

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