-
-
Notifications
You must be signed in to change notification settings - Fork 460
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
[ADD] New tool: oca-port #504
Conversation
Great! |
Live test on |
bdc355c
to
e6b58f5
Compare
Some PRs generated: https://github.com/OCA/wms/pulls?q=is%3Apr+is%3Aopen+oca-port-pr |
0f8b542
to
6941b21
Compare
In some local git configurations, origin is not configured as OCA (changed easily of course). But shouldn't be good to have also both remotes as argument ? |
And ten millions dollars question, shouldn't it be great to select PRs to port with a choice in case of multiple ? UPDATE: ooooh, didn't see further commands. Ok, I test furthermore |
Tested there : OCA/search-engine#98 Seems good for me |
@rousseldenis we can change the OCA remote with the About porting several PRs at the same time, I'm thinking about it. Often, one PR depends on a previous one (that's why the list of PRs are sorted by merge date), and I was thinking about proposing to the user to base the next PR on the branch of the last ported one, or make it by default... I really don't know how to handle that. It is difficult to know if a PR really depends on a previous one to avoid merge conflicts, and sometime PRs could be merged independently so introducing a dependency between them will just make more noise. |
1st of all: this feature is amazing. Thanks! My POV (as we discussed together): we should improve it to make it THE module migrator and fwd/back porter. I would:
As we discussed, using it for migration would require "only" to:
Finally one more feature could be: finding pending PRs before migrating. Meaning that the tool could say "hey, there's a pending PR that you could help review/merge that could be included in the migration". Or something like that. |
Thanks for this tool. Note that the guideline for the PR title is
Example: Thus, several comments about the PR title format.
I think both information should be on the main comment instead of being part of the title. We can suffix (or prefix) the PR with a similar formula like Odoo does, putting something short like |
@pedrobaeza is a PR title |
If you are parsing the module, why is it more difficult to parse the rest of the previous title? Anyways, in such case, I think it becomes more acceptable this mask: |
Because it's easier to wipe out things between brackets and version numbers from the original title, then adding our own suffixes afterward :)
EDIT: your suggestion is also a good one... |
I think it can be valuable to keep whole title for knowing the source, as you can do a patch in 12.0, and want to fw-port it to 13.0 and 14.0 (well, Other things that come to my mind now that I have dived a bit more in the PR:
|
We could do that indeed. It begins to be tricky if we want to forward port a PR on the two next versions (and we start on the previous one to make the next, so the title will be very long), but I think it is rare and we could still rename the PR afterwards. I'm also looking for existing PR created by this tool based on its name, but it seems GH is keeping previous PR values somewhere because I could still find the PR through GH API even with its old name. So let's do this simple for the moment with
Good idea, I'll do that.
If a conflict appears, the process will wait (as shown in the output example in the PR description), the user will have to resolve the conflict the normal way (finishing the resolution either with a
We discussed about that with @simahawk (see its comment above), later this tool could replace
Indeed, @gurneyalex whispers me this idea some weeks ago. Good catch about access rights, and sometime we have to resolve small conflicts (because of pre-commit changes for instance) so it's not easy to get a complete automatic flow here. |
d614e68
to
6bb3751
Compare
Last feature: $ oca-port-pr 13.0 14.0 --addon stock_available_to_promise_release --fork camptocamp
Fetch origin/13.0 from git://github.com/OCA/wms.git
Fetch origin/14.0 from git://github.com/OCA/wms.git
7 pull request(s) related to 'stock_available_to_promise_release' to port from origin/13.0 to origin/14.0
- PR #104 (https://github.com/OCA/wms/pull/104) [FIX] shopfloor: cluster picking, remove unavailable picking from validated batch:
By sebalix, merged at 2021-01-13T09:10:57Z
=> Not ported: ['stock_available_to_promise_release']
=> 1 commit(s) not (fully) ported
- PR #94 (https://github.com/OCA/wms/pull/94) [13.0][FIX] 'Release ready' based on the shipping policy of the order:
By sebalix, merged at 2021-01-13T11:53:40Z
=> Not ported: ['stock_available_to_promise_release', 'sale_stock_available_to_promise_release']
=> 1 commit(s) not (fully) ported
- PR #116 (https://github.com/OCA/wms/pull/116) [13.0] stock_available_to_promise_release: Change test case helper methods as classmethod:
By guewen, merged at 2021-01-14T14:08:03Z
=> Not ported: ['stock_available_to_promise_release']
=> 1 commit(s) not (fully) ported
- PR #106 (https://github.com/OCA/wms/pull/106) stock_available_to_promise_release: Fix quantity available to promise of released moves:
By jbaudoux, merged at 2021-01-14T14:40:11Z
=> Not ported: ['stock_available_to_promise_release']
=> 2 commit(s) not (fully) ported
- PR #126 (https://github.com/OCA/wms/pull/126) [13.0] stock_available_to_promise_release: Fix horizon - count released move outside horizon:
By jbaudoux, merged at 2021-01-21T12:48:02Z
=> Not ported: ['stock_available_to_promise_release']
=> 2 commit(s) not (fully) ported
- PR #173 (https://github.com/OCA/wms/pull/173) [13.0] Add shopfloor_workstation + sync w/ OCA/oca-addons-repo-template:
By simahawk, merged at 2021-02-12T01:48:18Z
=> Not ported: ['CONTRIBUTING.md', '.pylintrc-mandatory', 'README.md', 'stock_move_source_relocate_dynamic_routing', '.travis.yml', 'stock_reception_screen', 'stock_available_to_promise_release', 'oca_dependencies.txt', 'sale_stock_available_to_promise_release', '.prettierrc.yml', 'requirements.txt', '.pre-commit-config.yaml', '.github', '.copier-answers.yml', 'LICENSE', '.pylintrc', '.gitignore', 'shopfloor_workstation', 'setup']
=> 4 commit(s) not (fully) ported
- PR #174 (https://github.com/OCA/wms/pull/174) [13.0] stock_available_to_promise_release: Fix release of operation containing canceled moves:
By sebalix, merged at 2021-02-17T16:31:31Z
=> Not ported: ['stock_available_to_promise_release']
=> 2 commit(s) not (fully) ported
- Port PR #104 (https://github.com/OCA/wms/pull/104) [FIX] shopfloor: cluster picking, remove unavailable picking from validated batch...
Port it? [y/N]: y
Create branch oca-port-pr-104-from-13.0-to-14.0 from origin/14.0...
Apply ee928218552708328cd4bf0026045de4d25abf5f [FIX] stock_available_to_promise_release: fix flappy test...
Push branch 'oca-port-pr-104-from-13.0-to-14.0' to remote 'camptocamp'? [y/N]:
- Port PR #94 (https://github.com/OCA/wms/pull/94) [13.0][FIX] 'Release ready' based on the shipping policy of the order...
Port it? [y/N]: y
Use the previous PR #104 branch as base? [y/N]: y
Create branch oca-port-pr-94-from-13.0-to-14.0 from oca-port-pr-104-from-13.0-to-14.0...
Apply 5342da2d84b7d487815611e423be2a10e3280b94 [FIX] 'Release ready' based on the shipping policy of the order...
Push branch 'oca-port-pr-94-from-13.0-to-14.0' to remote 'camptocamp'? [y/N]:
- Port PR #116 (https://github.com/OCA/wms/pull/116) [13.0] stock_available_to_promise_release: Change test case helper methods as classmethod...
Port it? [y/N]: y
Use the previous PR #94 branch as base? [y/N]: y
Create branch oca-port-pr-116-from-13.0-to-14.0 from oca-port-pr-94-from-13.0-to-14.0...
Apply 6c7587a4188a4de24216110c19027c81c22d6a0d Change test case helper methods as classmethod...
Push branch 'oca-port-pr-116-from-13.0-to-14.0' to remote 'camptocamp'? [y/N]:
- Port PR #106 (https://github.com/OCA/wms/pull/106) stock_available_to_promise_release: Fix quantity available to promise of released moves...
Port it? [y/N]: y
Use the previous PR #116 branch as base? [y/N]: y
Create branch oca-port-pr-106-from-13.0-to-14.0 from oca-port-pr-116-from-13.0-to-14.0...
Apply 9ac87637c17d0291862027000e3124fbe8b93a01 stock_available_to_promise_release: Fix quantity available to promise of released moves...
Apply 0c57c0fc2d405b332bfc38affd557d3309bf5776 stock_available_to_promise_release: Fix moves for which available to promise is computed...
Push branch 'oca-port-pr-106-from-13.0-to-14.0' to remote 'camptocamp'? [y/N]:
- Port PR #126 (https://github.com/OCA/wms/pull/126) [13.0] stock_available_to_promise_release: Fix horizon - count released move outside horizon...
Port it? [y/N]: y
Use the previous PR #106 branch as base? [y/N]: y
Create branch oca-port-pr-126-from-13.0-to-14.0 from oca-port-pr-106-from-13.0-to-14.0...
Apply bc0d4dd7173acbed823fac71e93a9c43ba27e5f5 stock_available_to_promise_release: when a move is released, it must be counted in the stock previously promised even when it is outside horizon...
Apply dbb9d7c6c1494d2b58ac55cacd755f728b3d81d4 Fix update of quantity in tests...
Push branch 'oca-port-pr-126-from-13.0-to-14.0' to remote 'camptocamp'? [y/N]:
- Port PR #173 (https://github.com/OCA/wms/pull/173) [13.0] Add shopfloor_workstation + sync w/ OCA/oca-addons-repo-template...
Port it? [y/N]:
- Port PR #174 (https://github.com/OCA/wms/pull/174) [13.0] stock_available_to_promise_release: Fix release of operation containing canceled moves...
Port it? [y/N]: y
Branch oca-port-pr-174-from-13.0-to-14.0 already exists, recreate it?
(WARNING: you will lose the existing branch) [y/N]: y
Use the previous PR #126 branch as base? [y/N]: y
Create branch oca-port-pr-174-from-13.0-to-14.0 from oca-port-pr-126-from-13.0-to-14.0...
Apply d2bd0b89535e1d287fe6d0073029d416bb1aab85 Fix release of operation containing canceled moves...
Apply 7c95bdca1272b8c88c1044b9eb28dca93279c093 Fix release of operation containing canceled moves (test)...
🎉 Last PR processed! 🎉
Push branch 'oca-port-pr-174-from-13.0-to-14.0' to remote 'camptocamp'? [y/N]: y
PR(s) ported locally: #104, #94, #116, #106, #126, #174
Create a draft PR from 'oca-port-pr-174-from-13.0-to-14.0' to '14.0' against OCA/wms? [y/N]: y
PR created => https://github.com/OCA/wms/pull/270 |
221832d
to
70c5d05
Compare
9fb5274
to
eba823d
Compare
I've not had a chance to try it but I like the concept. |
I agree and TBH I'd prefer to have a separate project. Then MT could be just a bundle of them w/ some default conf maybe. |
@simahawk I agree, it'll also helps to show a quick user documentation in a README like the description of this PR. |
cool, I'll setup a new repo when you are ready |
The option to include this in module-migrator has been discarded? |
Maybe we need "OCA/contributor-tools" ?
…On 15/12/21 15:54, Pedro M. Baeza wrote:
The option to include this in module-migrator has been discarded?
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
<#504 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJQLJPQW5TOZAUZCOUUVGLURC24RANCNFSM5BF5ODGA>.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
What this tool does is a different work. It's not migrating modules where "migrating" means "adapt the code to a newer of older version of Odoo". It eases back/fwd porting of PRs and commits. I would keep it separated and we could integrate the migrator as a feature later. This feature could be installed optionally by the user like |
Thanks for this tool @sebalix. I have tested it against field-service for porting merged PR. Perf: BranchesDiff() takes ~25seconds before the first request. Issues:
Features I miss be able to target a precise (known) PR
Do not agree: having the original PR number in the name can help to track the progression.
There a distinction between (shared knowledge): the PR should not be merged (because it's false positive, a dead feature, not needed anymore...) and (local) I do not want to work on this PR now and do not want to be asked again next time. Conclusion: Really nice tool, I think it's good enough yet to be merged (or put on his own repo). |
This PR has the |
@hparfr thank you for your feedback!
10min, it's quite a lot even with a lot of commits to check 🤔 But yes it is sometimes a bit slow, I tried to reduce the time needed (even by switching from
It is expected, especially if the code base has changed a bit because of migration commits or, more often, new format rules with
Never encountered this one... or maybe because you didn't execute
Good idea, the tool should ask the user if they want to resume from there is there is a branch.
I miss it too, really a nice-to-have for the next time.
Indeed, so maybe add a comment when blacklisting a PR, and if we want to blacklist it locally, store this in a file that is |
Example of blacklisting to be shared OCA/edi#513 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally. Seems amazing!!!
Looks like an amazing tool!!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sebalix Is it scheduled to merge this ?
I think the plan was to move it to its own repo. |
Let's go ! 😃 |
Hi all, thanks for testing/reviewing 🙏 |
@sebalix better late than never https://github.com/OCA/oca-port 😉 |
This tool is now hosted on https://github.com/OCA/oca-port/, I'm closing this PR. |
Tool helping to port an addon or missing commits of an addon from one branch to another.
/!\ It requires Python 3.8. /!\
If the addon does not exist on the target branch, it will assist the user in the migration, following the OCA migration guide.
If the addon already exists on the target branch, it will retrieve missing commits to port. If a Pull Request exists for a missing commit, it will be ported with all its commits if they were not yet (fully) ported.
To check if an addon could be migrated or to get eligible commits to port:
To effectively migrate the addon or port its commits, use the
--fork
option:Migration of addon
The tool follows the usual OCA migration guide to port commits of an addon, and will invite the user to fullfill the mentionned steps that can't be performed automatically.
Output example:
Port of commits/Pull Requests
The tool will ask the user if he wants to open draft pull requests against the upstream repository.
If there are several Pull Requests to port, it will ask the user if he wants to base the next PR on the previous one, allowing the user to cumulate ported PRs in one branch and creating a draft PR against the upstream repository with all of them.
Output example (with --verbose):
Some details:
A
andB
, and an existing ported commit done in the past throughgit format-patch
only for one of the addons e.g.A
, so the commit will still be considered as eligible to be ported by this tool. If two ported commits exist (each updating one addon,A
thenB
), the commit will then be considered as already portedgit format-patch
, if a conflict appears, the process will wait the user to resolve it before continuing the cherry-pick, and so on.GITHUB_TOKEN
env var====
It is still a draft, for now I only tested it on
OCA/wms
on a bunch of addons.TODO:
git format-patch
instead ofgit cherry-pick
to port only changes of relevant addons to avoid to much conflicts if a commit has already been ported partially?Take into account renamed module (aAlready supported, example here=>
is used in the list of changed files in commit data, currently they are skipped)or):"installable": False
14.0-mig-$MODULE
(thanks @simahawk for the idea!), following the usual migration guide (so do not skip translation commits etc)oca-port
as it's not anymore a tool to port only PRs but entire module as well.--dev-branch
to work on the specified branch instead ofto_branch
(default)