-
-
Notifications
You must be signed in to change notification settings - Fork 801
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
[MIG/REM] purchase_order_line_packaging_qty: Makes it uninstallable #1672
[MIG/REM] purchase_order_line_packaging_qty: Makes it uninstallable #1672
Conversation
… from a procurement.
rename method and move to proper model
To keep it for records, make this module uninstallable as Odoo implemented all functionalities in core. Added tests to check that.
@MiquelRForgeFlow @LoisRForgeFlow @ChrisOForgeFlow I've added tests to check if all functionalities of this module have been implemented in Odoo. But made it uninstallable. People needs to 'just' update product_packaging > product_packaging_id field. The only thing that misses is the creation of purchase order line from procurement. But IMHO that one should not be part of this module and should be done in another one. I'll do it. |
@rousseldenis Thank you for the migration and the proposed approach. IMO we should not merge this one since it's deprecated and useless. The move of remaining logic into an other addon is a good idea. |
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.
Makes sense 👍
This PR has the |
@rousseldenis what to do here? |
/ocabot merge nobump |
This PR looks fantastic, let's merge it! |
@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1672-by-dreispt-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
@yajo ping /ocabot merge patch |
This PR looks fantastic, let's merge it! |
/ocabot migration purchase_order_line_packaging_qty |
@rafaelbn your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1672-by-rafaelbn-bump-patch. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
/ocabot merge patch |
Hey, thanks for contributing! Proceeding to merge this for you. |
@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1672-by-dreispt-bump-patch. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
It seems one test is failing:
Could you please rebase and see if it fails now, then fix it? |
@rousseldenis Thanx for migration |
I didn't read #1672 (comment) carefully. It seems to me like this should be handled in a migration script in https://github.com/OCA/openupgrade |
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
Keep it in this version uninstallable for records.