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

[14.0][FW]base_user_role: Groups-roles navigation #159

Merged

Conversation

GuillemCForgeFlow
Copy link
Contributor

@GuillemCForgeFlow GuillemCForgeFlow commented Apr 20, 2022

@ForgeFlow
This PR is a Forward Port of this IMP: #157

@OCA-git-bot
Copy link
Contributor

Hi @sebalix, @jcdrubay, @novawish,
some modules you are maintaining are being modified, check this out!

@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 14.0-fp-base_user_role-groups_roles_navigation branch from cd0a19d to 4a3df11 Compare April 20, 2022 13:05
Copy link
Contributor

@HaraldPanten HaraldPanten 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

@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 14.0-fp-base_user_role-groups_roles_navigation branch from 4a3df11 to 3eb37be Compare April 20, 2022 14:06
@GuillemCForgeFlow GuillemCForgeFlow changed the title [FW]base_user_role: Groups-roles navigation [14.0][FW]base_user_role: Groups-roles navigation Apr 20, 2022
@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 14.0-fp-base_user_role-groups_roles_navigation branch from 3eb37be to 91c3a8f Compare April 20, 2022 14:28
Copy link
Contributor

@sebalix sebalix left a comment

Choose a reason for hiding this comment

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

Can you cherry-pick the same commit done in https://github.com/OCA/server-backend/pull/158/commits ? It is to keep the same message, same timestamp, same author, etc, it helps to back/forward port commits from one branch to another.

A tool is in development to ease this process here: OCA/maintainer-tools#504 (but already quite stable).

@GuillemCForgeFlow
Copy link
Contributor Author

Can you cherry-pick the same commit done in https://github.com/OCA/server-backend/pull/158/commits ? It is to keep the same message, same timestamp, same author, etc, it helps to back/forward port commits from one branch to another.

A tool is in development to ease this process here: OCA/maintainer-tools#504 (but already quite stable).

Yes, I did that, but I had to edit the content of the commit as it was not working on v14 with the same content that the commit on v13 had.
So in that case, would you want me to cherry-pick and then apply the changes on a separate commit?
Also, in relation to that, I opened a fix with a silly change, if you could check it out: #161

@sebalix
Copy link
Contributor

sebalix commented Apr 21, 2022

@GuillemCForgeFlow it's up to you, if the changes are small, you can amend the cherry-picked commit, I often do that to avoid further merge conflicts when porting things to further versions (15.0, 16.0...), but if some changes are big enough, like a refactoring needed, a separate commit is nice.

@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 14.0-fp-base_user_role-groups_roles_navigation branch from 91c3a8f to bee32a1 Compare April 21, 2022 08:04
@GuillemCForgeFlow
Copy link
Contributor Author

Hi @sebalix , I don't know if that would work or not. I don't really know how to cherry-pick the commit: 212621305f80d6b9ae65903cd3e5ada76731c43f and then apply changes on the same commit without losing the timestamp, message, ...
Because if I apply changes I need to perform a fixup on the cherry-picked commit and then I lose the content.
How exactly should I do it?
And thanks in advance! 😄

@sebalix
Copy link
Contributor

sebalix commented Apr 21, 2022

@GuillemCForgeFlow as soon as you use git commit --amend or a fixup+squash in the original commit it's OK, don't worry about the timestamp, there are two of them in the commit and only the original one (authored one) matters.

@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 14.0-fp-base_user_role-groups_roles_navigation branch from bee32a1 to 418deb4 Compare April 21, 2022 08:21
@GuillemCForgeFlow
Copy link
Contributor Author

@sebalix Perfect, then now it should be all good! And thanks again for helping 😄

@sebalix sebalix added this to the 14.0 milestone Apr 21, 2022
@sebalix
Copy link
Contributor

sebalix commented Apr 21, 2022

@GuillemCForgeFlow it is still not good, the message of the commit was [IMP]base_user_role: Groups-roles navigation, you renamed it to [14][FW][IMP]base_user_role: Groups-roles navigation :) Nothing should change except the content, all other metadata as to stay the same.

@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 14.0-fp-base_user_role-groups_roles_navigation branch from 418deb4 to cb3025b Compare April 21, 2022 12:06
@GuillemCForgeFlow
Copy link
Contributor Author

@GuillemCForgeFlow it is still not good, the message of the commit was [IMP]base_user_role: Groups-roles navigation, you renamed it to [14][FW][IMP]base_user_role: Groups-roles navigation :) Nothing should change except the content, all other metadata as to stay the same.

Hope now it's okay! 😄

@sebalix
Copy link
Contributor

sebalix commented Apr 21, 2022

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-159-by-sebalix-bump-minor, awaiting test results.

@OCA-git-bot
Copy link
Contributor

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

@MiquelRForgeFlow MiquelRForgeFlow deleted the 14.0-fp-base_user_role-groups_roles_navigation branch April 21, 2022 15:03
SiesslPhillip pushed a commit to grueneerde/OCA-server-backend that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-backend (14.0)
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