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

[16.0][MIG] hr_timesheet_begin_end #566

Merged
merged 4 commits into from
May 24, 2023

Conversation

CRogos
Copy link
Contributor

@CRogos CRogos commented Feb 15, 2023

Migration using oca-port tool

@pedrobaeza
Copy link
Member

I think that tool doesn't serve for migrations. There are missing commits and attributions.

@CRogos
Copy link
Contributor Author

CRogos commented Feb 15, 2023

Yes I see what you mean, but maybe I've answered a question wrong... I'll try again.

@CRogos CRogos mentioned this pull request Feb 15, 2023
16 tasks
@CRogos
Copy link
Contributor Author

CRogos commented Feb 15, 2023

The problem is that this module was refactored in v15.
hr_timesheet_activity_begin_end became hr_timesheet_sheet_begin_end and hr_timesheet_begin_end

When migrating one of the new modules, it asks also to migrate hr_timesheet_activity_begin_end. But the result is not correct because this module should be deleted at the end. And it isn't.

The same happened also in v9. Before the name was different and the history is not preserved: d9472aa

image

@pedrobaeza
Copy link
Member

The blame is on 15.0, where the rename needed to be done deeply with this command:

git filter-branch --tree-filter 'if [ -d old_module_name ]; then mv old_module_name new_module_name; fi' HEAD
git rebase origin/branch

Not sure if we can handle here to retrieve with old name and then new name. It's too much to ask, and if authors do agree to keep it this way, it's OK for me.

@CRogos CRogos force-pushed the 16.0-mig-hr_timesheet_begin_end branch 2 times, most recently from 37d22b7 to b1785ef Compare February 15, 2023 16:02
@CRogos
Copy link
Contributor Author

CRogos commented Feb 15, 2023

The blame is on 15.0, where the rename needed to be done deeply with this command:

git filter-branch --tree-filter 'if [ -d old_module_name ]; then mv old_module_name new_module_name; fi' HEAD
git rebase origin/branch

Not sure if we can handle here to retrieve with old name and then new name. It's too much to ask, and if authors do agree to keep it this way, it's OK for me.

I did the split on v15, but I was not aware of this issue and git command.
I am also a little bit scared by the documentation of this command.

If there is anything I can improve, please leave a comment.

@CRogos CRogos marked this pull request as ready for review February 15, 2023 16:14
@CRogos CRogos force-pushed the 16.0-mig-hr_timesheet_begin_end branch 2 times, most recently from cfb2029 to 5d10e24 Compare March 16, 2023 10:29
@CRogos
Copy link
Contributor Author

CRogos commented Mar 16, 2023

@dalonsod committed the form change in a separate commit as suggested in #577 for easier backport to v15.

Copy link
Contributor

@dalonsod dalonsod left a comment

Choose a reason for hiding this comment

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

👍

@CRogos CRogos force-pushed the 16.0-mig-hr_timesheet_begin_end branch from 5d10e24 to 80ca325 Compare March 20, 2023 08:10
@CRogos CRogos force-pushed the 16.0-mig-hr_timesheet_begin_end branch from 203ce20 to efa1f3f Compare May 11, 2023 16:30
@CRogos
Copy link
Contributor Author

CRogos commented May 11, 2023

Added changes of #584

Copy link

@MohamedOsman7 MohamedOsman7 left a comment

Choose a reason for hiding this comment

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

Code & functional changes LGTM

@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). 🤖

@CRogos
Copy link
Contributor Author

CRogos commented May 17, 2023

@pedrobaeza

@pedrobaeza
Copy link
Member

Please squash 2 last commits together, as they don't add value being separated.

@dalonsod
Copy link
Contributor

I've made both commits (in separated PRs) for v14, that couldn't be cherry-picked in this particular case. So no problem squashing them.

@CRogos CRogos force-pushed the 16.0-mig-hr_timesheet_begin_end branch from efa1f3f to 8e8268f Compare May 22, 2023 10:38
Copy link

@MohamedOsman7 MohamedOsman7 left a comment

Choose a reason for hiding this comment

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

Code & Functional changes LGTM

@CRogos
Copy link
Contributor Author

CRogos commented May 24, 2023

Please squash 2 last commits together, as they don't add value being separated.

done.

@pedrobaeza
Copy link
Member

/ocabot migration hr_timesheet_begin_end

/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone May 24, 2023
@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-566-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit ff5247b into OCA:16.0 May 24, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at a2e4ab3. 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.

5 participants