Skip to content

[MIG] website_logo: Migration to 10.0 - #357

Merged
pedrobaeza merged 13 commits into
OCA:10.0from
Tecnativa:10.0-mig-website_logo
Jun 29, 2017
Merged

pedrobaeza merged 13 commits into
OCA:10.0from
Tecnativa:10.0-mig-website_logo

Conversation

@chienandalu

Copy link
Copy Markdown
Member

Website logo

Load a logo image to be used on website only. This allows to use an internal
company logo (for reports) and a different website logo.

cc @Tecnativa

StephanRozendaal and others added 12 commits June 23, 2017 09:35
Changes include:
 - update version number
 - make module installable
 - In company_view use an xpath expression to add logo configuration to
   company view.
In view, logo field should come after the 'Domain' group, which is the first group when
searching by XPath.
Tests are taken from commit: c1c9d53
…* Remove duplicated http send in favor of pass * Fix broken test for exception handling
* [FIX] website_logo:  show logo by domain

Use domain field to match a logo by website instead of the name field of the website
model.

* [FIX] website_logo update readme

The logo is now configured from 'Website admin' instead of 'Company settings'.

@pedrobaeza pedrobaeza 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.

Little changes, but working on runbot

Comment thread website_logo/README.rst Outdated

* go to your company and load a logo image in the website logo field
* Go to 'Website Admin'.
* In the configuration form of a website there is a category logo.

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.

s/category logo/"Logo" group

Comment thread website_logo/README.rst Outdated
Usage
=====

This addon supports multi-website. If no logo is defined a transparent image is

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.

comma after defined.

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.

Not really mandatory in English and it would kind of hinder the reading of the whole phrase...

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Native English speaker - can confirm, comma required

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.

Ok, I had the rule wrong on my mind (comma is not needed when the clause comes before the condition). Anyway, doesn't it feel like the sentence has more commas than needed? 😃

Comment thread website_logo/__manifest__.py Outdated
'category': 'Website',
'author': "Agile Business Group,Odoo Community Association (OCA)",
'author': "Agile Business Group, "
"Antiun Ingeniería S.L., "

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.

Remove Antiun

@chienandalu
chienandalu force-pushed the 10.0-mig-website_logo branch from b59993d to f9a9c56 Compare June 23, 2017 12:53
@chienandalu

Copy link
Copy Markdown
Member Author

@pedrobaeza changes done and squashed

@eLBati

eLBati commented Jun 23, 2017

Copy link
Copy Markdown
Member

Thanks @chienandalu
Commit ae19202 author is not recognized.
Is Stephan Rozendaal <stephan@west.nl> correct?

Then, 👍

@pedrobaeza

Copy link
Copy Markdown
Member

@eLBati that is because the GitHub user doesn't have that email address linked to its account.

@luismontalba luismontalba 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.

Tested on runbot

@eLBati

eLBati commented Jun 28, 2017

Copy link
Copy Markdown
Member

@pedrobaeza so he needs to configure the github account, to fix ae19202 , right?

@pedrobaeza

Copy link
Copy Markdown
Member

Exactly

@StephanRozendaal

Copy link
Copy Markdown

Hi, I added stephan@west.nl to my account.

@eLBati

eLBati commented Jun 28, 2017

Copy link
Copy Markdown
Member

@StephanRozendaal thanks

@lasley

lasley commented Jun 29, 2017

Copy link
Copy Markdown
Contributor

Looks good @chienandalu, thanks! Can you please squash f9a9c56 & 7dfd73c

FYI Travis was failing on one, but I'm pretty sure it was a timeout. Re-running it

@chienandalu
chienandalu force-pushed the 10.0-mig-website_logo branch from 7dfd73c to 4fc620b Compare June 29, 2017 15:01
@chienandalu

Copy link
Copy Markdown
Member Author

Thanks, @lasley 😄

@pedrobaeza
pedrobaeza merged commit a2e2fa8 into OCA:10.0 Jun 29, 2017
@pedrobaeza
pedrobaeza deleted the 10.0-mig-website_logo branch June 29, 2017 23:31
@pedrobaeza pedrobaeza mentioned this pull request Jun 29, 2017
35 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants