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

[17.0][ADD] hr_holidays_remaining_leaves #139

Open
wants to merge 20 commits into
base: 17.0
Choose a base branch
from

Conversation

xaviedoanhduy
Copy link
Contributor

@xaviedoanhduy xaviedoanhduy commented Aug 22, 2024

The original development of this module has been done by Mint System.
It can be found in: https://github.com/Mint-System/Odoo-Apps-HR/tree/16.0/hr_holidays_remaining_leaves
This module has been ported to the OCA with their agreement, cc @janikvonrotz

Supersedes: #127

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

  • README.md file shouldn't be touched.
  • The commit history contains other modules. Please squash story to reduce it and remove that noise.
  • I insist the name should be singular in the extra bits. hr_holidays_employee_remaining should serve.

@nilshamerlinck
Copy link

nilshamerlinck commented Aug 22, 2024

Hi @pedrobaeza

  • respectfully, I'm not convinced that hr_holidays_employee_remaining is a better name than hr_holidays_remaining_leaves for a module whose purpose is to "Show remaining leaves per employee in allocation overview"
  • in the end of the day what matters imho is to have a good distinctive and explicit name; I think the singular form naming rule was mainly for the odoo models (with some exceptions, to align on odoo inconsistencies), but not necessarily for the extra bits, especially if it ends up making the module's name confusing

@pedrobaeza
Copy link
Member

It was just a suggestion. What I try is to avoid the plurals, and let me tell you why the guideline about singular forms is there: this comes from the past (6.0, 6.1) where the module names where a total nightmare of mix of plurals, company names, etc, and it was very easy to misspell and commit typos while putting that names in the command line, in automation files, etc. A recurrent one is the s of the plural, specially when they are in the middle of the module name. That's why this rule exists. The exception is that Odoo doesn't follow these rules, and having hr_holidays, but extension modules being for example hr_holiday_extension will be weird, so we keep the plural in the prefix.

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Dec 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants