-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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] web_favicon: Migrated to 10.0 #459
Conversation
Hey @nikiwaibel, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
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.
Welcome to OCA!
Please remember to sign the CLA, and please could you take this chance to add usage instructions to the README?
Thanks!
</group> | ||
</page> | ||
<notebook position="inside"> | ||
<page string="Web Favicon"> |
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.
add name="favicon"
to <page>
please
@yajo CLA is signed and sent already. |
Can you tell me with which e-mail you sent it? |
|
CLA checked. Please change README.rst file to point to 10.0 version instead of 9.0 |
There's still at least one reference to 9.0 in the README (The "Try me on runbot" button). |
Should that be removed, or is it OK to just change the 9.0 to 10.0? On Nov 5, 2016 11:59, "Pedro M. Baeza" notifications@github.com wrote:
|
You have to change it to 10.0. |
done. sorry for rather slow progress. i am new to all this great runbot/codecov/travis automation thingies :). when is usually the main (10.0) branch updated? |
@nikiwaibel - The Runbot branches are built automatically upon push to the associated branch on Github. In the case of 10.0, it will be rebuilt every time a new PR is merged. |
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 submission! Always great to see new faces in OCA 😄
Please change from openerp
imports to the new v10 namespace from odoo
Also, the XML files should be upgraded from the old syntax. In order to do that, you basically just remove the <openerp>
and <data>
tags in favor of <odoo>
.
Example
<openerp>
<data>
<record id="view_company_form" model="ir.ui.view">
...
</record>
</data>
</openerp>
Becomes
<odoo>
<record id="view_company_form" model="ir.ui.view">
...
</record>
</openerp>
@lasley done. thanks for explanations! |
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 thanks @nikiwaibel
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 your contribution. I hope this is the first of a lot to come! Merging.
No description provided.