-
-
Notifications
You must be signed in to change notification settings - Fork 799
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
11.0 mig purchase allowed product #475
11.0 mig purchase allowed product #475
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.
Thanks for the migration. Code review - see comments inline.
purchase_allowed_product/README.rst
Outdated
Usage | ||
===== | ||
|
||
.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas | ||
:alt: Try me on Runbot | ||
:target: https://runbot.odoo-community.org/runbot/142/8.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 should update the target to 11.0
instead of removing the link
purchase_allowed_product/README.rst
Outdated
Configuration | ||
============= | ||
|
||
No extra configuration needed. |
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.
Remove empty sections
purchase_allowed_product/README.rst
Outdated
|
||
Known issues / Roadmap | ||
====================== | ||
|
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.
Remove empty sections
purchase_allowed_product/README.rst
Outdated
|
||
Funders | ||
------- | ||
|
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.
Remove empty sections
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.
It looks like nothing has changed in the code from previous version, so I assume it works 👍
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, thanks @hieulucky111
Can you please squash your migration commits for merge?
b8e416e
to
2ab735f
Compare
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 overall
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). | ||
|
||
from odoo import models | ||
|
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.
can we take the opportunity of the migration to fix this file's name (don't forget to change it in init.py)
from . import supplied_product_mixin | ||
from . import account_invoice | ||
from . import product | ||
from . import purchse_order |
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.
let's fix this typo: s/chse/chase/
…api.multi method! (OCA#475)
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.
Small Changes
'supplied by the supplier', | ||
'version': '11.0.1.0.0', | ||
'category': 'Accounting & Finance', | ||
'website': 'https://akretion.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.
@hieulucky111 Use URL as per this
@@ -0,0 +1,4 @@ | |||
# © 2017 Today Mourad EL HADJ MIMOUNE @ Akretion |
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.
@hieulucky111 IMO remove copyright in all init.py
This PR has been approved by two reviewers 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). 🤖 |
Please attend comments |
purchase_allowed_product/README.rst
Outdated
|
||
Usage | ||
===== | ||
|
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.
Preferably, the usage should mention the config steps
purchase_allowed_product/README.rst
Outdated
=========== | ||
|
||
Bugs are tracked on `GitHub Issues | ||
<https://github.com/OCA/hr/issues>`_. In case of trouble, please |
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.
wrong URL
2ab735f
to
0e298fa
Compare
0e298fa
to
fcb9ec8
Compare
@JayVora-SerpentCS can it be moved forward? Your comments were attendend |
This PR has the |
/ocabot merge |
Hey, thanks for contributing! Proceeding to merge this for you. |
It looks like something changed on |
Congratulations, your PR was merged at b5c717c. Thanks a lot for contributing to OCA. ❤️ |
No description provided.