Skip to content

[MIG] [9.0] website_breadcrumb - #216

Merged
rafaelbn merged 6 commits into
OCA:9.0from
khaeusler:9.0
Dec 13, 2016
Merged

rafaelbn merged 6 commits into
OCA:9.0from
khaeusler:9.0

Conversation

@khaeusler

Copy link
Copy Markdown

add the addon website_breadcrumb

@oca-clabot

Copy link
Copy Markdown

Hey @khaeusler, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

@pedrobaeza pedrobaeza changed the title add the addon website_breadcrumb [MIG] [9.0] website_breadcrumb Jun 22, 2016
@pedrobaeza

pedrobaeza commented Jun 22, 2016

Copy link
Copy Markdown
Member

Please sign the CLA and use the method explained on https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-9.0 instead of modifying commits with your authorship.

@rafaelbn rafaelbn added this to the 9.0 milestone Jun 22, 2016
while menu:
result.append(menu)
menu = menu.parent_id
return reversed(result) if revert else result

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

get all menu parents and include current if include_self is true and reverse the result in case revert is true

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.

That's what it does.

@rafaelbn

Copy link
Copy Markdown
Member

Hi @khaeusler , thanks for this contribution. Have you signed the CLA?

@khaeusler

Copy link
Copy Markdown
Author

yes

@rafaelbn

Copy link
Copy Markdown
Member

I have test it and there is not breadcrum in http://3150711-216-e4d94a.runbot1.odoo-community.org/blog/our-blog-1

@pedrobaeza CLA is OK?

@yajo could you take a look?

@khaeusler

khaeusler commented Jul 22, 2016

Copy link
Copy Markdown
Author

i see the breadcrum here http://3150711-216-e4d94a.runbot1.odoo-community.org/ in the DB 3150711-216-e4d94a-all

@rafaelbn

Copy link
Copy Markdown
Member

Hi @khaeusler , sorry. I mean that if you go to the blog there is not breadcrum,

@yajo

yajo commented Aug 1, 2016

Copy link
Copy Markdown
Member

Could you please squash all your personal commits? Their messages seem somewhat meaningless and having a single one that adapts to v9 will make it easier to evaluate the v8-v9 diff. Thanks 😊

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

Hi, I reviewed again and Breadcrum are missing if you navigate to the blog.

@yajo maybe you can clarify me, please

===================

This module allows you to have breadcrumbs in any page of your website.

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.

This doesn't happends, check image:

2016-10-04_16-06-39

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.

If an URL is not present in a menu (as indicated in the Configuration section), no breadcrumbs will display.

Definitely worse than OCA/website-cms#1, but much smaller too.

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

Almost fine, some things to do.

Comment thread website_breadcrumb/README.rst Outdated

.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas
:alt: Try me on Runbot
:target: https://runbot.odoo-community.org/runbot/186/8.0

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.

9.0

===================

This module allows you to have breadcrumbs in any page of your website.

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.

If an URL is not present in a menu (as indicated in the Configuration section), no breadcrumbs will display.

Definitely worse than OCA/website-cms#1, but much smaller too.

Comment thread website_breadcrumb/README.rst Outdated
:target: https://runbot.odoo-community.org/runbot/186/8.0

.. repo_id is available in https://github.com/OCA/maintainer-tools/blob/master/tools/repos_with_ids.txt
.. branch is "8.0" for example

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 this.

Comment thread website_breadcrumb/README.rst Outdated
<https://github.com/OCA/
website/issues/new?body=module:%20
website_breadcrumb%0Aversion:%20
8.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.

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.

Update this section to latest template.

Comment thread website_breadcrumb/__openerp__.py Outdated
"name": "Website Breadcrumbs",
"summary": "Let you have breadcrumbs in website pages",
"version": "9.0.1.0.0",
"category": "Uncategorized",

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.

Website

Comment thread website_breadcrumb/views/layout.xml Outdated
<!-- © 2015 Grupo ESOC Ingeniería de Servicios, S.L.U. - Jairo Llopis
License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -->

<openerp>

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.

<odoo>

Comment thread website_breadcrumb/views/templates.xml Outdated
<!-- © 2015 Grupo ESOC Ingeniería de Servicios, S.L.U. - Jairo Llopis
License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -->

<openerp>

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.

<odoo>

Comment thread website_breadcrumb/README.rst Outdated
@@ -0,0 +1,95 @@
.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg
:target: http://www.gnu.org/licenses/agpl-3.0-standalone.html
:alt: License: AGPL-3

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.

You can move to LGPL if you want.

@khaeusler

Copy link
Copy Markdown
Author

Hello,

i don't understand, how can i approve the change requests. I have only the option "Comment". On the other options, i got these messages:

Pull request authors can’t approve their own pull request

and

Pull request authors can’t request changes on their own pull request

I had taken the commits one by one so that the history is preserved.
Should i create a new Branch with one commit of the old website_breadcrumb code?

@pedrobaeza

Copy link
Copy Markdown
Member

You can't change that. People that have put that review should change it if they consider that this is correct now.

@yajo

yajo commented Nov 22, 2016

Copy link
Copy Markdown
Member

Yes, when changes are requested, you have to make those changes so we can approve them.

I had taken the commits one by one so that the history is preserved.
Should i create a new Branch with one commit of the old website_breadcrumb code?

Nope, it's your story that is to be squashed, then git push -f:

captura de pantalla de 2016-11-22 09-11-59

@khaeusler

Copy link
Copy Markdown
Author

next try ;-)

@khaeusler khaeusler closed this Nov 22, 2016
@pedrobaeza

Copy link
Copy Markdown
Member

Why do you close it?

@khaeusler

Copy link
Copy Markdown
Author

close??

@khaeusler

Copy link
Copy Markdown
Author

ups, wrong Button, sorry

@khaeusler khaeusler reopened this Nov 22, 2016
@yajo

yajo commented Nov 22, 2016

Copy link
Copy Markdown
Member

I think you have it mostly. You have to keep the commits that are not yours untouched, but squash those that are yours. Please review #216 (comment) again.

@khaeusler

khaeusler commented Nov 22, 2016

Copy link
Copy Markdown
Author

what is wrong on this changeset?
d1cd215

Jairo Llopis and others added 4 commits November 26, 2016 18:47
 - increase the version from 8.0.1.0.0 to 9.0.1.0.0
 - fix the runbot url to 9.0
 - remove <data> in the templates
 - replace <openerp> with <odoo> in the templates
@pedrobaeza

Copy link
Copy Markdown
Member

I have amended commits for you.

@lasley lasley left a comment

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.

LGTM, code no test

@khaeusler

Copy link
Copy Markdown
Author

@pedrobaeza
thanks, but how did you do this?

@lasley

lasley commented Nov 27, 2016

Copy link
Copy Markdown
Contributor

@khaeusler - after the fact may have been a bit different, but the process of splitting commits based on module is part of our upgrade instructions - https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-9.0#howto

@khaeusler

Copy link
Copy Markdown
Author

oh i see, i used a wrong branch name, should i create a new branch with the name 9.0-mig-website_breadcrumb?

@lasley

lasley commented Nov 27, 2016

Copy link
Copy Markdown
Contributor

No worries on the branch name (although heed for next time). The important part is:

git format-patch --stdout origin/9.0..origin/8.0 -- <module path> | git am -3

which creates a patch of the difference between 8.0 and 9.0, but only for the module path that you define. This splits the commits while preserving ownership, which is likely the process Pedro used to fix this branch.

@khaeusler

Copy link
Copy Markdown
Author

@lasley
thanks for the answer

@pedrobaeza

Copy link
Copy Markdown
Member

@khaeusler, have you signed the CLA? I can't find it. Who was the sender of the CLA e-mail?

Comment thread website_breadcrumb/README.rst Outdated

To configure the shown breadcrumbs, you need to:

* Go to *Settings > Configuration > Website Settings > Configure website

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.

Please update the path of this configuration putting: Go to *Website Admin > Configuration* and click on "Configure website menus"

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.

I have updated myself

@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 comment, but it's working correctly.

@pedrobaeza

Copy link
Copy Markdown
Member

Waiting for CLA.

@khaeusler

Copy link
Copy Markdown
Author

@pedrobaeza
I've sent you an email

@khaeusler

Copy link
Copy Markdown
Author

@pedrobaeza
Have you found the CLA?

@lasley

lasley commented Dec 13, 2016

Copy link
Copy Markdown
Contributor

@rafaelbn - Can you double check that your comments have been attended to please?

@rafaelbn

Copy link
Copy Markdown
Member

Rebuilding runbot for testing

@rafaelbn

Copy link
Copy Markdown
Member

My comments and doubts from #216 (comment) are pending. Check:

2016-12-13_18-03-33

@rafaelbn

Copy link
Copy Markdown
Member

OK, I guess is because dependencies. So I merge.

@rafaelbn
rafaelbn merged commit 76d36fb into OCA:9.0 Dec 13, 2016
@pedrobaeza pedrobaeza mentioned this pull request Dec 13, 2016
34 tasks
@oca-clabot

Copy link
Copy Markdown

Hey @khaeusler,
We acknowledge that the following users have signed our Contributor License Agreement:

Appreciation of efforts,
OCA CLAbot

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.

8 participants