-
-
Notifications
You must be signed in to change notification settings - Fork 650
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][MIG] product_expiry_simple #tryagain #1271
[14.0][MIG] product_expiry_simple #tryagain #1271
Conversation
6e72384
to
f43b817
Compare
/ocabot migration product_expiry_simple |
7e7c650
to
ea59e8a
Compare
makepot: "true" | ||
name: test with Odoo | ||
- container: ghcr.io/oca/oca-ci/py3.6-ocb14.0:latest | ||
exclude: "product_expiry_simple,product_expiration_date" | ||
name: test with OCB |
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.
Please @rousseldenis could you help to fix configuration to get it works
by comparison of https://github.com/OCA/server-tools/blob/14.0/.github/workflows/test.yml#L36
it seems ok, but in real not
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.
Yes, I saw your tries.
But, the problem here I think is the mutual exclusion in the manifest. I think the product_expiry module is installed in the first step as required by other modules.
I think you should exclude them also and put in the rebel modules: stock_mass_scrap, stock_move_quick_lot. Could you try ?
@sbidoul Any advice?
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.
Looking at the error I see "product_expiration_date does not exist". But indeed this module does not exist 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.
You're right, madre mia, I shouldn't work after 22h, caramba !
The migration issue (#708) has been updated to reference the current pull request. |
And @bealdav the question at 10.000 dollars. Isn't there a mean to make this (the module) configurable to allow it being installed along with product_expiry ? |
97e0efa
to
db1d6df
Compare
@rousseldenis I suppose you suggest a new module product_expiry_simple_but_not_so_easy_finally ? |
No, I wanted to see if both can coexist. Maybe by hiding the standard fields when installing this |
Yes, I understood, sorry for my joke ;-) But you're right, in this case runboat fails because of |
What are the options to make runboat works even if product_expiry_simple is not installed ? |
At the moment, runboat does not support conflicting modules that prevent simultaneous installation of all modules in the repo. Suggestions are welcome in sbidoul/runboat#36 |
Probably, but runboat install addons of the whole repo with dependency. |
I don't believe in a solution where product_expiry_simple depend on product_expiry ; I think it will bring more problems than solutions. The goal of product_expiry_simple is to have a easier/simpler drop-in replacement to product_expiry. Then we need to adapt the OCA tests system to allow the development of such alternative modules. |
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. |
db1d6df
to
51ee61c
Compare
@sbidoul @rousseldenis could you add |
@bealdav Following sbidoul/runboat#36, is this needing the rebel module configuration ? |
I suppose this not needed, then we can merge this PR as soon as there reviews. Could you approve the PR @rousseldenis ? |
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.
LGTM
/ocabot merge nobump |
On my way to merge this fine PR! |
Congratulations, your PR was merged at c52c818. Thanks a lot for contributing to OCA. ❤️ |
@bguillot @bealdav @rousseldenis this has broken runboat for other PRs, eg: #1383 |
Oops, I forgot sbidoul/runboat#36 to deal with rebel modules. |
Yes. This will prevent testing on runboat but not merges. We will need to manage it during OCA days. Remotely :-( |
@rousseldenis any plan to fix runboat for 14? thank you! |
@bealdav gentle reminder as we're out of runboat on v14 :) |
The problem is that I don't really understand runboat machinery to make a pertinent PR. ;-( |
supersed #934