Skip to content

[11.0][MIG] website_analytics_piwik - #404

Merged
pedrobaeza merged 6 commits into
OCA:11.0from
onesteinbv:11_mig_website_analytics_piwik
Jan 15, 2018
Merged

pedrobaeza merged 6 commits into
OCA:11.0from
onesteinbv:11_mig_website_analytics_piwik

Conversation

@astirpe

@astirpe astirpe commented Dec 21, 2017

Copy link
Copy Markdown
Member

Porting of module website_analytics_piwik to V11

@astirpe astirpe mentioned this pull request Dec 21, 2017
38 tasks
@pedrobaeza pedrobaeza added this to the 11.0 milestone Dec 21, 2017

@simahawk simahawk left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

just code review: LGTM

@simahawk

Copy link
Copy Markdown

I'd squash translation commits all in one and - if time allows - prefix some commits (like "fix coding style" and such) w/ the module name 😉

@@ -1,4 +1,3 @@
# -*- coding: utf-8 -*-

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why have you removed the encoding?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because utf-8 is default encoding for Python 3

@astirpe

astirpe commented Jan 13, 2018

Copy link
Copy Markdown
Member Author

Translation commits squashed together

@astirpe

astirpe commented Jan 15, 2018

Copy link
Copy Markdown
Member Author

Can we merge this one?

@yung-wang yung-wang left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested on our piwik server with odoo v11 and it works. Please merge this PR.

@hbrunn hbrunn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could you add a migration script that sets the flag for existing installations which have an ID set?

@astirpe

astirpe commented Jan 15, 2018

Copy link
Copy Markdown
Member Author

@hbrunn Done!

@astirpe

astirpe commented Jan 15, 2018

Copy link
Copy Markdown
Member Author

Squashed

@pedrobaeza

Copy link
Copy Markdown
Member

Merging

@pedrobaeza
pedrobaeza merged commit 62c32cc into OCA:11.0 Jan 15, 2018
@astirpe
astirpe deleted the 11_mig_website_analytics_piwik branch January 15, 2018 14:29
@hbrunn

hbrunn commented Jan 15, 2018

Copy link
Copy Markdown
Member

so I kept this tab open so wait for runbot come back positive...

@astirpe
astirpe restored the 11_mig_website_analytics_piwik branch February 6, 2018 07:53
@astirpe
astirpe deleted the 11_mig_website_analytics_piwik branch February 6, 2018 08:02
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.

7 participants