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

[9.0] disable autoinstall #575

Merged
merged 4 commits into from
Dec 27, 2016
Merged

Conversation

hbrunn
Copy link
Member

@hbrunn hbrunn commented Dec 19, 2016

This is a cherry pick of #133 and #166, with my own addition of not autoinstalling auth_signup. No upstream PR for obvious reasons

@hbrunn hbrunn added this to the 9.0 milestone Dec 19, 2016
Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Surprisingly, website_portal does not seem to depend on portal so changing the dependencies of website_portal_sale seems just right. Now, a more pedantic person than me would suggest not to add a dependency on portal but instead change the existing dependency of website_portal_sale on sale into a dependency on 'portal_sale' but technically, that works out the same.

@pedrobaeza
Copy link
Member

Can you please explain why do you need to add such dependency on website_portal_sale?

@StefanRijnhart
Copy link
Member

@pedrobaeza it has a dependency on xmlids from the portal_sale module, which is not a problem because portal is autoinstalled up until this change and portal_sale is installed as a glue module automatically. Now that portal is no longer autoinstalled, the dependency has to be made explicit otherwise the website_portal_sale module will break.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation. Indeed, I remember now that my colleague has made this PR trying to improve the situation: odoo#14777

@hbrunn
Copy link
Member Author

hbrunn commented Dec 27, 2016

two approves, 7 days - can one of you do the honors @pedrobaeza @StefanRijnhart ?

@pedrobaeza pedrobaeza merged commit 56e08e1 into OCA:9.0 Dec 27, 2016
@yajo
Copy link
Member

yajo commented Dec 28, 2016

Hi everyone, just letting you know this broke OCB build of OCA/website.

@hbrunn
Copy link
Member Author

hbrunn commented Dec 28, 2016

then website should fix its dependencies

@yajo
Copy link
Member

yajo commented Dec 28, 2016

Not actually dependencies, other things, but it will be fixed soon and linked here, don't worry.

@pedrobaeza
Copy link
Member

Fixed here: OCA/website#306

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.

5 participants