[9.0][MIG][web_shortcut] #564
Conversation
There was a problem hiding this comment.
@tarteo Thank you!
See my comments on the code.
Could you also replace all the occurrences of web_shortcuts -> web_shortcut in every *.po file? This way the translation of all the already translated terms will be applied.
File web_shortcuts.pot can be removed (is not useful).
web_shortcut/README.rst
Outdated
| =========== | ||
|
|
||
| Bugs are tracked on `GitHub Issues | ||
| <https://github.com/OCA/162/issues>`_. In case of trouble, please |
There was a problem hiding this comment.
Must be https://github.com/OCA/web/issues here
web_shortcut/__openerp__.py
Outdated
| 'category': 'Tools', | ||
| 'name': 'Shortcut Menu', | ||
| 'version': '9.0.1.0.0', | ||
| 'category': 'Web', | ||
| 'author': "OpenERP SA,Odoo Community Association (OCA)", | ||
| 'website': 'http://openerp.com', |
There was a problem hiding this comment.
Please add 'license': 'AGPL-3', in the manifest, otherwise it will take the LGPL licence which is the default for Odoo 9
web_shortcut/__openerp__.py
Outdated
| 'category': 'Tools', | ||
| 'name': 'Shortcut Menu', | ||
| 'version': '9.0.1.0.0', | ||
| 'category': 'Web', | ||
| 'author': "OpenERP SA,Odoo Community Association (OCA)", |
There was a problem hiding this comment.
Please replace the double quotes with single quotes, so this line will be consistent with the rest of the manifest
|
@astirpe Thanks for the review! I've made the changes you suggested. |
web_shortcut/models/ir_ui_menu.py
Outdated
| def unlink(self): | ||
| res = super(IrUiView, self).unlink() | ||
| shortcuts = self.env['web.shortcut'].search([('menu_id', '=', False)]) | ||
| for shortcut in shortcuts: |
There was a problem hiding this comment.
this could be just shortcuts.unlink() ;)
| ############################################################################*/ | ||
|
|
||
| odoo.define('web.shortcut', function(require) { | ||
| var widget = require('web.Widget'), |
There was a problem hiding this comment.
you should keep classes' names CamelCase, ie:
var Widget = require('web.Widget'),
|
@simahawk I applied your suggestions. Thanks! |
| @@ -1,3 +1,4 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
@elicoidal I've replaced the headers. Thanks!
| mission is to support the collaborative development of Odoo features and | ||
| promote its widespread use. | ||
|
|
||
| To contribute to this module, please visit https://odoo-community.org. |
There was a problem hiding this comment.
You need to add this disclaimer: https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#backporting-odoo-modules
There was a problem hiding this comment.
This is not a back port (8.0 -> 9.0). Do I still have to apply these conventions?
There was a problem hiding this comment.
Yes please, just replace backport with port
| ------------ | ||
|
|
||
| * Adrien Peiffer | ||
| * Dennis Sluijk <d.sluijk@onestein.nl> |
|
@elicoidal could you give a look to the headers? |
|
Can we merge, please? |
|
@elicoidal your approval is still pending, this PR is on hold since a long time |
Previously web_shortcuts