-
-
Notifications
You must be signed in to change notification settings - Fork 671
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]Migrate stock_picking_show_return from v11 to v12 #477
[MIG]Migrate stock_picking_show_return from v11 to v12 #477
Conversation
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.
IMHO You have to respect commit history following migration guide: https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-12.0#technical-method-to-migrate-a-module-from-110-to-120-branch
============================== Show returns on stock pickings ============================== This module enables one tab in the pickings that contains returns made for that picking.
The returned picking must be obtained from the picking of the iteration, not the set of pickings to be calculated.
OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex
For avoiding errors when other modules add required fields on affected models
e0d3553
to
4f478fb
Compare
Commit history has been added |
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.
@bizzappdev Thanks for migration. Would you please correct minor changes. Other than it's LGTM
|
||
.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas | ||
:alt: Try me on Runbot | ||
:target: https://runbot.odoo-community.org/runbot/154/11.0 |
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.
Should be 12.0
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 can use README by fragments and generate this automatically: https://github.com/OCA/maintainer-tools/#readme-generator
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.
Done
"author": "Tecnativa, " | ||
"Odoo Community Association (OCA)", | ||
"category": "Warehouse Management", | ||
"website": "https://www.tecnativa.com", |
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.
Should be https://github.com/OCA/stock-logistics-workflow
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.
Done
suggested change are done |
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.
Code Review 👍
@bizzappdev Use new README by fragment system, copying https://github.com/OCA/maintainer-tools/tree/master/template/module/readme and editing or removing the needed sections. You can then run locally oca-gen-addon-readme if you have OCA/maintainer-tools installed and you want to pre-generate the README.rst file (preferred), or keep the previous README.rst file. |
class StockPicking(models.Model): | ||
_inherit = 'stock.picking' | ||
|
||
returned_ids = fields.Many2many( |
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.
We should use table name (Eg. relation='returned_picking_relation'
) explicitly to prevent sharing same table name (In case one have m2m with same object)
Related PR on Github: odoo/odoo#29376 and odoo/odoo#29323
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.
@Swapnesh-SerpentCS As it is non-store field it will not create any database table
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.
Improve Code
stock_picking_show_return/README.rst
Outdated
|badge1| |badge2| |badge3| |badge4| |badge5| | ||
|
||
================================= | ||
Display returns on stock pickings |
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.
@bizzappdev Remove it, Two time added
@@ -0,0 +1,6 @@ | |||
================================= | |||
Display returns on stock pickings |
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.
same as
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.
Done
@bizzappdev squash commits |
sorry but I am not good in Squashing. might be the merger will do. |
You can see the guide for that: https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests Tell me if you have any doubt. This can't be done at merge time unless the merger downloads locally the branch, do the squashing and force push over your PR. If no luck, I'll do it. |
@bizzappdev follow this command/steps: |
45c6c48
to
a41fe7e
Compare
thanks @pedrobaeza for pointing doc. I had only doubt that last time when I forced push the PR was automatically closed that is why I was worrying. I have force pushed the commit with squash. Please check |
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.
Code review. Minor changes
a5f759f
to
4bca2ca
Compare
[11.0][NEW] stock_secondary_unit: New module for get stock in a secondary unit
Migrate from v11 to v12
#457