Skip to content
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

Question about stock_move_location_dest_constraint_base and why it depends on a module not merged #972

Closed
BT-rmartin opened this issue Sep 17, 2020 · 5 comments

Comments

@BT-rmartin
Copy link

Dear community,

I recently realized about stock_move_location_dest_constraint_base depends on a module not yet merged in stock-logistics-warehouse.

This is fixed by this dependency, but I would like to better understand that. That will download that branch that contains that module, but also all the others from stock-logistics-warehouse to the folder dependencies in Travis. How is then deciding if it should apply the version of those addons or the one from main stock-logistics-warehouse? I guess it's related to the order of the addons path.

Wouldn't be better not to depend on something that is not yet merged?

Thanks for your clarifications in advance

@grindtildeath @dreispt @LoisRForgeFlow

@grindtildeath
Copy link
Contributor

Hello @BT-rmartin ,

It is indeed better not to depend on an unmerged PR, as the dependent PR should have been merged before #696 was merged.

The unmerged dependent PR is #707 which backports the new v13.0 implementation of putaways in v12.0.
However, both these modules are tagged with maturity level Alpha, and we will not maintain stock_move_location_dest_constraint_base as we used another implementation in v13.0...

Therefore we can either:

From my POV, it would even make more sense to do both and have #707 merged and revert the merge #696

cc @jgrandguillaume @rousseldenis

@rousseldenis
Copy link
Contributor

@grindtildeath @jgrandguillaume

Be careful when you want to merge Alpha modules. The dependencies must be merged before. Although it could lead to inconsistencies.

Indeed, reverting #696 makes sense. As I understand you develop now in v13. So, the logic would tend to backport those modules.

@BT-rmartin
Copy link
Author

Hi,

Thanks for replying. Then if you agree #696 can be reverted, you can go for it, In my opinion #707 can be merged as well as at least it doesn't add strange dependencies. But I'm nobody here to make those decisions. Could you @grindtildeath @jgrandguillaume @rousseldenis please get in touch with whoever can decide on that and proceed asap?

Thanks in advance

@grindtildeath
Copy link
Contributor

I just opened revert PR #973 and I guess we can fast track it so it will close this issue.

#707 still needs reviews before being merged anyway.

@BT-rmartin
Copy link
Author

Closing the issue as the revert PR #973 was merged. Thanks for your support guys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants