11.0 mig hr public holidays #383
Conversation
|
There's already another PR for this module, but none of both respect commit history following migration guide: https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-11.0 |
|
@pedrobaeza The first PR, i don't do following migration guide, sorry for my mistake. So, I removed it and created a new PR following Migration guide. |
|
No worries, and welcome here! I hope this is the first of a long list 😉 You can just start again deleting the branch in local, creating a new one with the same name, following the guide, and finally force pushing again to your repository. Then, this PR will be changed with all the commits. |
hieulucky111
force-pushed
the
11.0-mig-hr_public_holidays
branch
2 times, most recently
from
39d6c71
to
0e94cd8
Compare
hieulucky111
force-pushed
the
11.0-mig-hr_public_holidays
branch
from
0e94cd8
to
f986261
Compare
…Management modules
[MIG] hr_public_holidays
[MIG] hr_public_holidays/hr_holidays_compute_days: port to 10.0
* [ADD] public holidays: add a wizard to create public holidays for the following year * [REF] odoolint improvements * [REF] adjustments for flake8 and pylint test * Move public holidays menu under Leaves menu. (OCA#290) * [ADJ] hr_public_holidays: take into account the need of several sub-menus * [ADD] Allow the creation of public holidays from a specific template for a specific year, and display a UserError for templates including February 29th. * [REF] removing useless comments * [ADJ] hr_public_holidays: Improving wizard display * [REF] Removing tab characters
* hr_public_holidays: use group 'hr_holidays.group_hr_holidays_manager' * hr_public_holidays: Update README.rst
[MIG] hr_public_holidays: Migration to 11.0
hieulucky111
force-pushed
the
11.0-mig-hr_public_holidays
branch
from
90403e0
to
d7154bf
Compare
|
Superseeded by #401 |
|
@feketemihai @pedrobaeza I don't think it is really encouraging for new contributors:
From the distance, it gives some feeling of abuse of authority from @feketemihai :/ Thanks in advance for sharing your thoughts, or rules I might miss. I have no expectations about the output of this PR, and I am sure that the reasons behind the choice to close are reasonable. But I think it is in the best interest of OCA to encourage new contributors and therefore it seems important to recognize them as Contributors. Thanks |
|
@jcdrubay It's not my intention to discourage new contributors, but @pedrobaeza guided @hieulucky111 in this PR, the history was ok, but he just changed to version in the manifest file, it was a job of other 5 minutes to remove the coding lines, and to retest the module, i am involved in a project quite urgent and i need the modules migrated, and in most of the cases the response time in a PR is more than one week, that's why i choose to superseed the PR, and if you're feeling that i am abusing, it's your thoughts, i cannot change them... |
|
Thanks for taking the time to answer. I don't see how your action helped your purpose as anyway the code can't be merge in 11.0 in a mater of hours so you will need to work with your personal branch anyway. Please count on us to be reactive on any PR we create. |
|
The normal procedure for superseeding a PR is first to ask in the original PR (this one) if they are willing to continue the work or if they prefer to leave the gap to the new contributor. Said that, in this case my last changes request was more than one month ago without any answer, so it hasn't been exactly an abuse, but not the more elegant way. I hope @hieulucky111 understands all of these reasons and don't be discouraged to contribute more. |
|
Thanks for your message Pedro, I have tried to follow the guidelines after your comment and you can see the activity in the PR after your comment. I apologize if it was not perfect, but I did not understand that anything additional was requested from me. Please let me know what I should have done more so I can take into account in my other opened PR and in future PR. I am not discouraged :) |
|
@hieulucky111 sorry, indeed you did some activity, but it wasn't notified automatically to us as it was a rebase + forced push. Please take into account for future times that you need to throw a comment saying that you have made the proper changes. Then, @feketemihai, what was your problem with this PR? Any way, if everyone agrees, let's continue as the goal I think it's to have the more number of modules migrated and to not duplicate efforts. What you can do for joining both efforts is to cherry-pick commit from @hieulucky111 and do the rest of the changes needed on top of that one. Do you agree? |
[BSSFL-507] No downpayment line on DN / Invoice
[MIG] hr_public_holidays: Migration to 11.0