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

[10.0] [MIG] help_online #526

Merged
merged 4 commits into from
Mar 8, 2017
Merged

Conversation

lmignon
Copy link
Contributor

@lmignon lmignon commented Jan 13, 2017

Migration of Help_online from 9.0.

@pedrobaeza pedrobaeza mentioned this pull request Jan 13, 2017
59 tasks
@lmignon lmignon force-pushed the 10.0-mig-help_online-lmi branch from a52f7fd to bcb43b7 Compare January 13, 2017 12:44
@lmignon lmignon force-pushed the 10.0-mig-help_online-lmi branch from bcb43b7 to 49c738a Compare January 13, 2017 15:21
@lmignon lmignon changed the title 10.0 mig help online lmi [10.0] [MIG] help_online Jan 17, 2017
@lasley lasley added this to the 10.0 milestone Jan 18, 2017
Copy link
Contributor

@lasley lasley left a comment

Choose a reason for hiding this comment

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

Thanks @lmignon. Few non-blocking comments, but otherwise awesome module both in terms of code and functionality!

@@ -1,5 +1,5 @@
<?xml version='1.0' encoding='UTF-8' ?>
<openerp>
<odoo>
Copy link
Contributor

Choose a reason for hiding this comment

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

<odoo noupdate="1"> & can remove data tag

@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<openerp>
<odoo>
<data>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can remove data tag

<field name="context">{"search_default_website":1}</field>
</record>

<menuitem id="menu_help_main" name="Help Online" groups="help_online_group_writer"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice if the web icon was set to the application icon here something like this

Copy link
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

👍 Code review

Copy link

@njeudy njeudy left a comment

Choose a reason for hiding this comment

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

Code review and test

@lasley
Copy link
Contributor

lasley commented Mar 6, 2017

@lmignon - This is approved, but lacking my non-blocking comments. Were you planning on attending to them before merge?

@lmignon
Copy link
Contributor Author

lmignon commented Mar 8, 2017

@lasley Done
image

Always use the '__export__' prefix when generating the xmlid of pages exported by the export wizard to avoid autmomatic removal of these pages by Odoo on system update (-u)
@lmignon
Copy link
Contributor Author

lmignon commented Mar 8, 2017

PLZ don't squash the commits on merge.

@lasley lasley merged commit 6cb007b into OCA:10.0 Mar 8, 2017
@lasley
Copy link
Contributor

lasley commented Mar 8, 2017

Thanks @lmignon. Merged, no squash!

@sbidoul sbidoul deleted the 10.0-mig-help_online-lmi branch April 14, 2017 10:06
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.

6 participants