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

Module 'purchase_requisition_line_analytic' should not really depend on 'account_analytic_plans' #44

Merged
merged 2 commits into from
Feb 18, 2015

Conversation

JordiBForgeFlow
Copy link

Should only depend on 'account_analytic'. The dependence on 'account_analytic_plans' installs this module, which can be a problem for companies that really don't use it.

If purchase requisitions should really make use of analytic plans, then I suggest a new module on top of the existing called 'purchase_requisition_line_analytic_plan'.

Regards,
Jordi.

…on module 'account_analytic_plans' but on 'analytic'.
@nhomar
Copy link
Member

nhomar commented Dec 13, 2014

@hbto Can you send me a +1 please. I think @jbeficent has a point here.

@jbeficent can you complement this mp with the second module that you are commenting in order to make only one merge with the split, please.

@JordiBForgeFlow
Copy link
Author

@nhomar Module 'purchase_requisition_line_analytic_plan' that I was not mentioning really does not exist. I was just pointing out that it would need to be created, if needed, instead of adding the dependency of 'account_analytic_plans' in 'purchase_requisition_line_analytic'.

…o that a user following a requisition can view the requisition that he follows.
@JordiBForgeFlow
Copy link
Author

Hi, I just added a new proposed change to this PR. @nhomar @hbto can you review & merge if you agree?

@JordiBForgeFlow
Copy link
Author

@moylop260 Can you review this one also?

@moylop260
Copy link
Contributor

@jbeficent
You have two changes.
First one: purchase_requisition_for_everybody
Second one: account_analytic_plans

Two changes LGTM.
But is better if you next fix is in 2 PR.

moylop260 added a commit that referenced this pull request Feb 18, 2015
Module 'purchase_requisition_line_analytic' should not really depend on 'account_analytic_plans'
@moylop260 moylop260 merged commit 13f405f into Vauxoo:7.0 Feb 18, 2015
moylop260 added a commit to vauxoo-dev/addons-vauxoo that referenced this pull request Feb 18, 2015
Module 'purchase_requisition_line_analytic' should not really depend on 'account_analytic_plans'
moylop260 added a commit that referenced this pull request Feb 18, 2015
@moylop260
Copy link
Contributor

@jbeficent
Merged
Thanks for you contribution.

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.

None yet

3 participants