-
-
Notifications
You must be signed in to change notification settings - Fork 288
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] [IMP] purchase_sale_inter_company: sync pickings between companies #507
[14.0] [IMP] purchase_sale_inter_company: sync pickings between companies #507
Conversation
78aba10
to
54dff83
Compare
@renda-dev Maybe you are interested in #508. |
54dff83
to
a23f78c
Compare
27cd537
to
1827dc7
Compare
@MiquelRForgeFlow would you mind reviewing this one? We'll add tests soon. |
Then do you mind if I wait for the tests? haha |
798bca3
to
b23630f
Compare
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.
Functional ok!
@MiquelRForgeFlow @AaronHForgeFlow ready for review! |
b23630f
to
1709c38
Compare
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.
Code LGTM
@AaronHForgeFlow shall we merge? :) |
4c08f4e
to
0957cd6
Compare
@AaronHForgeFlow we got to cover with tests cases added by our changes, but not the ones from original PR. Do you think it's good for merge? Thank you! |
@JordiBForgeFlow can this be merged? AFAICS you're the only active PSC in this repo |
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.
code review LGTM
…roducts Imagine you have a customization that enables the split of a stock move into several moves (distributed in several pickings). Thus, we need to handle this case.
0957cd6
to
8859792
Compare
Rebase done |
@pedrobaeza try again please? |
/ocabot merge major |
What a great day to merge this nice PR. Let's do it! |
@pedrobaeza any idea on how to get out of this situation? It happened before but I can't remember what we did to fix it. |
This has happened before if the setup folders are manually created instead of created by pre-commit. |
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.
In case you want to do some change, in addition do this.
8859792
to
261ca0a
Compare
Sorry for bothering you, but could you please try again? @pedrobaeza |
Rebel modules is conflicting in these cases. It's better to find the compatible way, and at the end that conflict will happen in your DBs: /ocabot merge nobump |
Hey, thanks for contributing! Proceeding to merge this for you. |
Congratulations, your PR was merged at 6710442. Thanks a lot for contributing to OCA. ❤️ |
Thanks a lot for the support @pedrobaeza @MiquelRForgeFlow ! |
Migration of #362
Added some fixes and migration to v14
Disclaimer:
I've added some things in the CI's tests matrix because this module can't run it's test with
stock_intercompany
installed, since the purposes of these two modules are quite similar.Also, I can't exclude
stock_intercompany
in this module'smanifest.py
because the runboat would not even start.I'm totally open to suggestion on how we can handle this situation.