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

[ADD] support analytic plans #98

Merged
merged 2 commits into from Sep 9, 2015

Conversation

hbrunn
Copy link
Member

@hbrunn hbrunn commented Aug 31, 2015

The small refactoring should make it simpler to add more things like that or to change the matching logic completely in an inheriting module

@pedrobaeza
Copy link
Member

You can already inherit the list and add elements. account_invoice_merge module already do it in https://github.com/OCA/account-invoicing/blob/8.0/account_invoice_merge_payment/models/account_invoice.py#L30, and you break it with you change.

@hbrunn
Copy link
Member Author

hbrunn commented Aug 31, 2015

@pedrobaeza doing it this way is a very bad idea. As soon as account_invoice_merge_payment has a static folder, this key is added when the addon is in the addons path without it being installed. At least, this should be protected by means of _register_hook or such, and we should encourage using extension mechanisms that respect the difference between "module is loaded python wise" and "module is installed in the database"

@hbrunn
Copy link
Member Author

hbrunn commented Aug 31, 2015

@pedrobaeza I you agree, I'll include fixing this module in the PR

@pedrobaeza
Copy link
Member

OK, then please change the extension mechanism to the function then.

@hbrunn
Copy link
Member Author

hbrunn commented Aug 31, 2015

@pedrobaeza done

@legalsylvain
Copy link
Contributor

👍 Thanks.

@pedrobaeza
Copy link
Member

👍

@sbidoul
Copy link
Member

sbidoul commented Aug 31, 2015

@adrienpeiffer can you have a look at this one?

@adrienpeiffer
Copy link
Contributor

LGTM (Code review and functionnal test). I'll update #92 according changes in this one. 👍

@sbidoul
Copy link
Member

sbidoul commented Sep 9, 2015

👍

sbidoul added a commit that referenced this pull request Sep 9, 2015
…_plans

[ADD] support analytic plans in account_invoice_merge
@sbidoul sbidoul merged commit b1e8bc1 into OCA:8.0 Sep 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants