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.

Sign up for GitHub

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

Jump to bottom

10.0 mig sale layout multi company #83

Merged
merged 7 commits into from
May 21, 2018
Merged

10.0 mig sale layout multi company #83

merged 7 commits into from
May 21, 2018

Conversation

SimoRubi
Copy link
Member

Hi,
I've migrated this module from v8 to v10, please check.
Thanks

@SimoRubi SimoRubi mentioned this pull request Jan 18, 2018
Closed
8 tasks
@SimoRubi SimoRubi force-pushed the 10.0-mig-sale_layout_multi_company branch from 8df73df to b59afd8 Compare February 28, 2018 11:32
@tafaRU
Copy link
Member

tafaRU commented Mar 26, 2018

@SimoRubi, please check Travis.

@SimoRubi SimoRubi force-pushed the 10.0-mig-sale_layout_multi_company branch 2 times, most recently from 3690fee to dcc4fcd Compare March 26, 2018 15:16
@SimoRubi SimoRubi force-pushed the 10.0-mig-sale_layout_multi_company branch from dcc4fcd to e65ee3f Compare April 20, 2018 16:34
eLBati and others added 2 commits April 21, 2018 18:46
@eLBati @SimoRubi
ADD sale_layout_multi_company
8189919 
This module extends the functionality of sale_layout module adding support for multi company
@oca-transbot @SimoRubi
OCA Transbot updated translations from Transifex
e5e35fd 
OCA Transbot updated translations from Transifex
@SimoRubi SimoRubi force-pushed the 10.0-mig-sale_layout_multi_company branch 2 times, most recently from 094d6d1 to b101acc Compare April 21, 2018 18:32
pedrobaeza
pedrobaeza reviewed Apr 23, 2018
View reviewed changes
sale_layout_multi_company/views/sale_layout_view.xml Outdated
<field name="inherit_id" ref="sale.report_configuration_form_view"/>
<field name="arch" type="xml">
<field name="sequence" position="after">
<field name="company_id" groups="base.group_multi_company" widget="selection"/>
Copy link
Member

Choose a reason for hiding this comment

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

I miss here company_ids field, which is the main aim of the base_multi_company module.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pedrobaeza thanks for the review, done!

@SimoRubi SimoRubi force-pushed the 10.0-mig-sale_layout_multi_company branch 2 times, most recently from f048ac8 to 2a0e8bd Compare April 23, 2018 17:08
@SimoRubi SimoRubi force-pushed the 10.0-mig-sale_layout_multi_company branch from 871011c to 12ede7d Compare May 21, 2018 08:19
@SimoRubi SimoRubi force-pushed the 10.0-mig-sale_layout_multi_company branch from 12ede7d to 48c9222 Compare May 21, 2018 08:42
@SimoRubi
Copy link
Member Author

@OCA/intercompany-maintainers is this ok to be merged?

elicoidal
elicoidal suggested changes May 21, 2018
View reviewed changes
base_multi_company/__manifest__.py Outdated
@@ -5,7 +5,7 @@
{
'name': 'Multi Company Base',
'summary': 'Provides a base for adding multi-company support to models.',
'version': '10.0.1.0.1',
'version': '10.0.1.0.2',
Copy link

Choose a reason for hiding this comment

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

Shouldnot this be 10.0.2.0.0 as there is a new constraint and migration script involved?

Copy link
Member Author

@SimoRubi SimoRubi May 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

done in 625cab3

sale_layout_multi_company/__init__.py Outdated
@@ -0,0 +1,5 @@
# -*- coding: utf-8 -*-
# Copyright 2016 Lorenzo Battistini - Agile Business Group
Copy link

Choose a reason for hiding this comment

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

copyright/license not needed in init.py files

Copy link
Member Author

@SimoRubi SimoRubi May 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

done in a8482b1

sale_layout_multi_company/__manifest__.py
@@ -0,0 +1,23 @@
# -*- coding: utf-8 -*-
# Copyright 2016 Lorenzo Battistini - Agile Business Group
Copy link

Choose a reason for hiding this comment

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

probably 2016-2018

Copy link
Member Author

@SimoRubi SimoRubi May 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

done in a8482b1

sale_layout_multi_company/__manifest__.py Outdated
"summary": "Multi company features for sale_layout",
"version": "10.0.1.0.0",
"category": "Sales Management",
"website": "https://www.agilebg.com",
Copy link

Choose a reason for hiding this comment

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

use the OCA target repository URL

Copy link
Member Author

@SimoRubi SimoRubi May 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

done in a8482b1

@SimoRubi
Copy link
Member Author

@elicoidal thanks for the review, I've implemented your requested changes

@SimoRubi SimoRubi force-pushed the 10.0-mig-sale_layout_multi_company branch from 2ccf60c to 625cab3 Compare May 21, 2018 09:36
elicoidal
elicoidal reviewed May 21, 2018
View reviewed changes
Copy link

@elicoidal elicoidal left a comment

Choose a reason for hiding this comment

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

almost there

sale_layout_multi_company/__manifest__.py
"summary": "Multi company features for sale_layout",
"version": "10.0.1.0.0",
"category": "Sales Management",
"website": "https://github.com/OCA/multi-company/tree/10.0/"
Copy link

Choose a reason for hiding this comment

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

just https://github.com/OCA/multi-company

Copy link
Member Author

@SimoRubi SimoRubi May 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I added the module's complete address because I think it's nice for the user to be redirected directly to the module's page; it should be ok according to https://github.com/OCA/maintainer-tools/blob/master/template/module/__openerp__.py#L9

@elicoidal elicoidal merged commit 872d225 into OCA:10.0 May 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pedrobaeza pedrobaeza pedrobaeza left review comments

@eLBati eLBati eLBati approved these changes

@tafaRU tafaRU tafaRU approved these changes

@elicoidal elicoidal elicoidal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@SimoRubi @tafaRU @eLBati @pedrobaeza @elicoidal @oca-transbot