[18.0][MIG] sale_order_line_remove #3888
Conversation
ArnauCForgeFlow
force-pushed
the
18.0-mig-sale_order_line_remove
branch
from
ce0fef7 to
c3fc8cc
Compare
ArnauCForgeFlow
force-pushed
the
18.0-mig-sale_order_line_remove
branch
from
a9d3988 to
0b52b4c
Compare
LoisRForgeFlow
left a comment
LoisRForgeFlow
left a comment
There was a problem hiding this comment.
Functional test, works as expected 👍
BhaveshHeliconia
left a comment
BhaveshHeliconia
left a comment
There was a problem hiding this comment.
Functional review LGTM!
|
This PR has the |
.copier-answers.yml
Outdated
| org_slug: OCA | ||
| rebel_module_groups: | ||
| - sale_packaging_default | ||
| - sale_order_line_remove |
There was a problem hiding this comment.
@ArnauCForgeFlow @LoisRForgeFlow And what about introducing an activation parameter that allows to not break standard behaviour? That allows to activate/deactivate behaviour ... and preserve planet with less CI jobs 😄
There was a problem hiding this comment.
@rousseldenis If you install this module you want the behavior, the parameter is redundant in my opinion. So I guess it is a trade-off between code simplicity and CI efficiency.
There was a problem hiding this comment.
That is the eternal debate. But having a way to deactivate/activate a behavior change in the interface is a good practice IMHO
There was a problem hiding this comment.
@rousseldenis I let you the final call as PSC of the repo. Do you want us to add that parameter?
There was a problem hiding this comment.
Sure!
@ArnauCForgeFlow could you take care of this?
ArnauCForgeFlow
force-pushed
the
18.0-mig-sale_order_line_remove
branch
from
0b52b4c to
c53564a
Compare
|
@rousseldenis done! |
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at 09945ac. Thanks a lot for contributing to OCA. ❤️ |
6 participants
Migration to version 18.0 + add sale_order_line_remove as rebel module as mentioned in the comments from the original pr: #3632 (comment).
Supersedes: #3632