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] resource_calendar_get_days #138

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/15.0/resource_calendar_get_days
This module has been ported to the OCA with their agreement, cc @janikvonrotz

- Supersedes: #129

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.

Same questions as the other PR about Mint grant / CLA sign and the module name.

@leemannd
Copy link

Hello @janikvonrotz , your formal acceptation into the PR is welcome.
@xaviedoanhduy When opening such new PRs would it be possible to gently ask a review from the author of the module?

@pedrobaeza
Copy link
Member

Another question is about the module name, as singular forms are advisable: https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#modules

Copy link

@janikvonrotz janikvonrotz left a comment

Choose a reason for hiding this comment

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

I was able to install the module and did a test similar to the tests.py:

image

Everything works as expected.

@janikvonrotz
Copy link

Same questions as the other PR about Mint grant / CLA sign and the module name.

I signed the CLA and support the migration.

@pedrobaeza
Copy link
Member

Yes, great. The module name is the only remaining thing.

@leemannd
Copy link

Hello @pedrobaeza I must confess that on this case, the naming using the plural dosen't bother me.
The override done within the scope of this module is about the field and methods: days & _get_days_data

Maybe a better name could be resource_calendar_get_days_data_company , which in the other hand is super long.
Do you have a proposition on this topic?

@pedrobaeza
Copy link
Member

Well, the description on the README is not very related: Show remaining leaves per employee in allocation overview., so I have no clue...

@pedrobaeza pedrobaeza dismissed their stale review August 27, 2024 09:51

Requirements fulfilled

@pedrobaeza pedrobaeza added this to the 17.0 milestone Aug 27, 2024

|badge1| |badge2| |badge3| |badge4| |badge5|

Show remaining leaves per employee in allocation overview.

Choose a reason for hiding this comment

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

This is supposed to be "Calculate number of leave days relative to company working hours."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @janikvonrotz, sorry for the omission, i have updated the description

@xaviedoanhduy xaviedoanhduy force-pushed the 17.0-mig-resource_calendar_get_days branch from 4afb7de to 8f3552d Compare August 28, 2024 08:54
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
@github-actions github-actions bot closed this Feb 2, 2025
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