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

[15.0][MIG] stock_available_immediately: Migration to 15.0 #1588

Merged
merged 22 commits into from
Dec 30, 2022

Conversation

victoralmau
Copy link
Member

Superseed #1507

Migration to 15.0

@Tecnativa TT36990

nbessi and others added 21 commits December 22, 2022 10:44
(lp:c2c-addons/6.1  rev 28.1.10)
…PL-3 with author agreement, improved help messages and pep8

(lp:c2c-addons/6.1  rev 45.1.8)
[UPG] Upgraded to version 8, fixed references to new 8.0  views and  moved fields that were in  product.product to product.template

[fix] remove duplicate view and correct view name

[UPG][FIX] added outgoing field XML, that was in the base stock field in 7.0.

[UPG] stock available immediately, corrected the calculation method of
immediately_usable_qty to take in accountthe sign change in outgoing_qty
(from negative to positive) in version 8.

[FLAKE8]

[FIX] renaming of a class, comment removing, useless code.

[UPD] move out from unported to 8 for update

[fix] remove duplicate view and correct view name

[UPG][FIX] added outgoing field XML, that was in the base stock field in 7.0.

[FIX] renaming of a class, comment removing, useless code.

[fix] remove duplicate view and correct view name

[UPG][FIX] added outgoing field XML, that was in the base stock field in 7.0.

[UPD] move out from unported to 8 for update

[fix] remove duplicate view and correct view name

[UPG][FIX] added outgoing field XML, that was in the base stock field in 7.0.

[FIX] renaming of a class, comment removing, useless code.

[UPD] move out from unported to 8 for update

[fix] remove duplicate view and correct view name

[UPG][FIX] added outgoing field XML, that was in the base stock field in 7.0.

[FIX] renaming of a class, comment removing, useless code.

[UPD] move out from unported to 8 for update

[fix] remove duplicate view and correct view name

[UPG][FIX] added outgoing field XML, that was in the base stock field in 7.0.

[FIX] renaming of a class, comment removing, useless code.

[fix] remove duplicate view and correct view name

[UPG][FIX] added outgoing field XML, that was in the base stock field in 7.0.

[UPD] move out from unported to 8 for update

[fix] remove duplicate view and correct view name

[UPG][FIX] added outgoing field XML, that was in the base stock field in 7.0.

[FIX] renaming of a class, comment removing, useless code.
Generic module to compute the stock quantity available to promise using several implementations.
stock_available_immediatly is changed to become the first optional implementation.
Cherry pick of commit 0b060f6 from the v7 branch

[IMP] stock_available* uses new API

[IMP] READMEs and TODOs

Cherry-picked from v7 at 8add4be

Conflicts:
	__unported__/stock_available_mrp/__openerp__.py
	stock_available/__openerp__.py
	stock_available_immediately/__openerp__.py
…uct and

product.template, now takes in account variants and correctly displays value.
[FLAKE8]

Removing duplicate modules and moving README.rst into __unported__
OCA Transbot updated translations from Transifex
OCA Transbot updated translations from Transifex
* fix the dependencies for the computed field

* use api.multi instead of api.one to avoid calling
  super()._immediately_usable_qty in a loop (this improves perfs on a tree view
  display)
There are cases where we dot NOT want to simply sum the quantities of all the
variants. For example when dealing with manufacturing capacities, we may have
to chose between variants because we can't make ALL of them with the same
components.

So instead of a simple non-modular implementation, we'll let each module define
his own implementation of how to compute the product template's quantity
available for sale.

Conflicts:
	stock_available/__openerp__.py
	stock_available_immediately/__openerp__.py

OCA Transbot updated translations from Transifex
optimize stock computation by avoiding to call useless compute

OCA Transbot updated translations from Transifex
Due to merge mess, we need to restore current code this way
* New README by fragments
* Bump manifest version
* Adapted tests

[UPD] Update stock_available_immediately.pot
[UPD] Update stock_available_immediately.pot

[UPD] README.rst
@rousseldenis
Copy link
Contributor

@victoralmau Don't forget that fair play here is to ask first the original author if we can switch. Thanks

@pedrobaeza
Copy link
Member

We asked on November the 14th for the needed changes with no answer: #1507 (comment), so I think it's already fair play.

…les (sale_stock_available_info_popup for example).

TT36990
@victoralmau victoralmau marked this pull request as ready for review December 22, 2022 10:09
@victoralmau
Copy link
Member Author

Tests are now green, could you review it?

@rousseldenis
Copy link
Contributor

We asked on November the 14th for the needed changes with no answer: #1507 (comment), so I think it's already fair play.

Yes. I don't like putting in code something that is for tests only.

@rousseldenis
Copy link
Contributor

Better is to put this one in rebel module as it is modifying normal field value

@pedrobaeza
Copy link
Member

I don't like such technique and prefer this one, and it's reasonable to put such code. Tests are code as well, so we are not doing something exceptional. Putting rebel modules complicates everything. If you prefer to do it, please do it on your own modules. We will stick to this one.

@rousseldenis
Copy link
Contributor

If you prefer to do it, please do it on your own modules. We will stick to this one.

This is not how community is working. It's worth the common good that should enforce. Rebel modules is an OCA flow, so we should use it and not putting code that is not necessary. Don't do that.

@pedrobaeza
Copy link
Member

Rebel modules is a tool like the one we are using. You can't enforce such one that brings a lot of more load to the CI and wait times. This is only adding 3 lines of code.

@rousseldenis
Copy link
Contributor

Rebel modules is a tool like the one we are using. You can't enforce such one that brings a lot of more load to the CI and wait times. This is only adding 3 lines of code.

We don't use Travis anymore that was used to be very long. Github actions jobs are very fast compared to (3 minutes here). So, this is not an argument.

As already said, adding code that is not necessary for a particular context is not wanted as for quality we want on OCA.

Moreover, you can see in the whole history of available modules that we have been involved in their developments.

@pedrobaeza
Copy link
Member

4 minutes x 2 = 8 minutes

vs

3 lines of code.

We prefer the second one, sorry.

@pedrobaeza
Copy link
Member

And it's not only that, but then 2 separate commits of PO updates that will generate as well 2 new builds on merge, apart from other problems like the one faced in OCA/product-attribute#1169 (comment)

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from discussions 👍 for me

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ocabot migration stock_available_immediately
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Dec 30, 2022
@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 15.0-ocabot-merge-pr-1588-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

The migration issue (#1278) has been updated to reference the current pull request.
however, a previous pull request was referenced : #1507.
Perhaps you should check that there is no duplicate work.
CC : @glitchov

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot OCA-git-bot merged commit e1ed3f3 into OCA:15.0 Dec 30, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 52a0a3d. Thanks a lot for contributing to OCA. ❤️

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

Successfully merging this pull request may close these issues.