Minor bugfix: Added missing composer dependency (ServiceManager) to Math package #5186

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@dol
Contributor

dol commented Sep 29, 2013

The Zend\Math package uses the ServiceManager. But the composer.json requirements don't define this dependency.
This PR fixes this issue.

@samsonasik

This comment has been minimized.

Show comment
Hide comment
@samsonasik

samsonasik Sep 29, 2013

Contributor

I think it should be on 'suggest' dependency. We still can use Zend\Math\BigInteger\Adapter\Bcmath directly.

Contributor

samsonasik commented Sep 29, 2013

I think it should be on 'suggest' dependency. We still can use Zend\Math\BigInteger\Adapter\Bcmath directly.

@dol

This comment has been minimized.

Show comment
Hide comment
@dol

dol Sep 29, 2013

Contributor

I agree, that you could use Zend\Math\BigInteger\Adapter\Bcmath directly. The convenience of BigIntiger::factory() and the build-in Zend\ServiceMangager\AbstractPluginManager is that you don't have to decide witch adapter to use.
Without the ServiceManager dependency I have to duplicate the code of decision making of using Zend\Math\BigInteger\Adapter\Bcmath or Zend\Math\BigInteger\Adapter\Gmp.
I'd like to use this package for a math project and I don't want to deal with different calculation adapters. The missing dependency blocks me from using this package.
The examples in the ZF2 documentation won't work out of the box without the ServiceManager.

I would rather add ext-bcmath and ext-gmp (e.g) to the 'suggest' section.

Btw:
There was recently a poll on the composer-dev mailinglist about the 'suggest' section and when to display.

Contributor

dol commented Sep 29, 2013

I agree, that you could use Zend\Math\BigInteger\Adapter\Bcmath directly. The convenience of BigIntiger::factory() and the build-in Zend\ServiceMangager\AbstractPluginManager is that you don't have to decide witch adapter to use.
Without the ServiceManager dependency I have to duplicate the code of decision making of using Zend\Math\BigInteger\Adapter\Bcmath or Zend\Math\BigInteger\Adapter\Gmp.
I'd like to use this package for a math project and I don't want to deal with different calculation adapters. The missing dependency blocks me from using this package.
The examples in the ZF2 documentation won't work out of the box without the ServiceManager.

I would rather add ext-bcmath and ext-gmp (e.g) to the 'suggest' section.

Btw:
There was recently a poll on the composer-dev mailinglist about the 'suggest' section and when to display.

@samsonasik

This comment has been minimized.

Show comment
Hide comment
@samsonasik

samsonasik Sep 30, 2013

Contributor

ok, so it up to the zf maintainer to agree or not :). btw, i think 'component' is the better word than 'module' :D

Contributor

samsonasik commented Sep 30, 2013

ok, so it up to the zf maintainer to agree or not :). btw, i think 'component' is the better word than 'module' :D

@dol

This comment has been minimized.

Show comment
Hide comment
@dol

dol Sep 30, 2013

Contributor

@samsonasik Thanks you for the critics about this issue. A second opinion is always good. btw: I changed the title from 'module' to 'package' due the naming taken from here.

Contributor

dol commented Sep 30, 2013

@samsonasik Thanks you for the critics about this issue. A second opinion is always good. btw: I changed the title from 'module' to 'package' due the naming taken from here.

@weierophinney

This comment has been minimized.

Show comment
Hide comment
@weierophinney

weierophinney Oct 23, 2013

Member

@dol I agree with @samsonasik here -- this is a suggested dependency, not a required dependency, as you can use selected functionality from the component without the ServiceManager package installed. While I understand that the examples in the doc will not work without the SM, the documentation also typically assumes you've installed the entire framework.

Change the package to a suggestion, and add in the string "if using the BigInteger::factory functionality". I'd also add "ext-bcmath" and "ext-gmp" as suggestions.

Member

weierophinney commented Oct 23, 2013

@dol I agree with @samsonasik here -- this is a suggested dependency, not a required dependency, as you can use selected functionality from the component without the ServiceManager package installed. While I understand that the examples in the doc will not work without the SM, the documentation also typically assumes you've installed the entire framework.

Change the package to a suggestion, and add in the string "if using the BigInteger::factory functionality". I'd also add "ext-bcmath" and "ext-gmp" as suggestions.

weierophinney added a commit that referenced this pull request Jan 3, 2014

Merge pull request #5186 from dol/bugfix/missing-servicemanager-depen…
…dency

Minor bugfix: Added missing composer dependency (ServiceManager) to Math package

weierophinney added a commit that referenced this pull request Jan 3, 2014

[#5186] incorporate feedback
- Moved servicemanager to suggested dependency
- Added ext-bcmath and ext-gmp as suggested dependencies

weierophinney added a commit that referenced this pull request Jan 3, 2014

Merge branch 'hotfix/5186' into develop
Forward port #5186

Conflicts:
	library/Zend/Math/composer.json

@ghost ghost assigned weierophinney Jan 3, 2014

@dol dol deleted the dol:bugfix/missing-servicemanager-dependency branch Jan 6, 2014

weierophinney added a commit to zendframework/zend-math that referenced this pull request May 15, 2015

Merge pull request zendframework/zendframework#5186 from dol/bugfix/m…
…issing-servicemanager-dependency

Minor bugfix: Added missing composer dependency (ServiceManager) to Math package

weierophinney added a commit to zendframework/zend-math that referenced this pull request May 15, 2015

[zendframework/zendframework#5186] incorporate feedback
- Moved servicemanager to suggested dependency
- Added ext-bcmath and ext-gmp as suggested dependencies

weierophinney added a commit to zendframework/zend-math that referenced this pull request May 15, 2015

Merge branch 'hotfix/5186' into develop
Forward port zendframework/zendframework#5186

Conflicts:
	library/Zend/Math/composer.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment