Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

dol
Copy link
Contributor

@dol 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
Copy link
Contributor

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

@dol
Copy link
Contributor Author

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
Copy link
Contributor

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

@dol
Copy link
Contributor Author

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
Copy link
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.

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

Minor bugfix: Added missing composer dependency (ServiceManager) to Math package
weierophinney added a commit that referenced this pull request Jan 3, 2014
- 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
Forward port #5186

Conflicts:
	library/Zend/Math/composer.json
@ghost ghost assigned weierophinney Jan 3, 2014
@dol dol deleted the bugfix/missing-servicemanager-dependency branch January 6, 2014 14:23
weierophinney added a commit to zendframework/zend-math that referenced this pull request May 15, 2015
…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
- 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
Forward port zendframework/zendframework#5186

Conflicts:
	library/Zend/Math/composer.json
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants