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

ServiceManager v2-v3 compatibility #95

Merged
merged 3 commits into from
Feb 18, 2016
Merged

ServiceManager v2-v3 compatibility #95

merged 3 commits into from
Feb 18, 2016

Conversation

kynx
Copy link
Contributor

@kynx kynx commented Feb 13, 2016

This builds on #86 adding ServiceManager v2/v3 compatibility.

The v2 tests will fail pending zendframework/zend-validator#51

@@ -17,9 +17,15 @@ matrix:
- php: 5.5
env:
- EXECUTE_CS_CHECK=true
- php: 5.5
env:
- SERVICE_MANAGER_VERSION="^2.7.5"
Copy link
Member

Choose a reason for hiding this comment

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

I disagree with this. The correct is to test lowest version (composer --prefer-lowest) and newest versions. All versions released in the middle are expected to be compatible and fulfill the API contract.

When we should to test specific versions? When the code HAS an specific logic branch testing if some feature is available i.e. (component_version >= 5.0) ? new API : old API. And this thing should be do with only 1 PHP version and not 5.5. and 5.6 whcih make me the following question?

Why PHP 7 and HHVM are not duplicated?

I think the test matrix proposed in this PR is not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum, OK, didn't know about --prefer-lowest (not my preference often ;) I've just been following what I've seen @weierophinney do in PRs for zend-i18n etc.

I noticed @ezimuel has done it slightly differently, with a SERVICE_MANAGER_V2=(true|false). So something like:

install:
  - if [[ $SERVICE_MANAGER_V2 != 'true' ]]; then travis_retry composer install --no-interaction --ignore-platform-reqs ; fi
  - if [[ $SERVICE_MANAGER_V2 == 'true' ]]; then travis_retry composer install --no-interaction --ignore-platform-reqs --prefer-lowest ; fi

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

s/composer install/composer update/ - install doesn't do --prefer-lowest

@kynx
Copy link
Contributor Author

kynx commented Feb 13, 2016

Nice one @Maks3w ! That makes flipping between versions when testing locally one heck of lot easier too :)

@kynx
Copy link
Contributor Author

kynx commented Feb 17, 2016

@Maks3w : doesn't look like the --prefer-lowest is going to work.

The fail is due to the minimum symfony/file version specified by php-cd-fixer, which is 2.1. The notPath() method it's failing on was introduced in 2.2, and even that is pretty ancient.

@weierophinney
Copy link
Member

@kynx and @Maks3w — I tried it out with a couple of components this past week, and ran into similar issues; development tools were causing failures by composer to resolve dependencies. Specifying the upper and/or lower bounds as separate tests seems to work consistently and without issue.

@Maks3w
Copy link
Member

Maks3w commented Feb 18, 2016

@weierophinney Probably is an issue with the cached dependencies. Could you detail what issues did you had so I could investigate?

Recently I have refactored a Symfony Test suite from to test every Supported version (Currenlty 4) to only to test lowest and newest with the only issue of reset the cache.

Previous: https://travis-ci.org/Maks3w/FR3DLdapBundle/builds/108739742
Current: https://travis-ci.org/Maks3w/FR3DLdapBundle/builds/109023062

@weierophinney weierophinney added this to the 2.6.0 milestone Feb 18, 2016
@weierophinney weierophinney self-assigned this Feb 18, 2016
*/
public function __construct(ConfigInterface $configuration = null)
public function __construct(ContainerInterface $parentLocator, array $config = [])
Copy link
Member

Choose a reason for hiding this comment

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

First argument must also accept null for full BC with v2; I'll update that when merging. (Discovered this the hard way with zend-view 2.6.0.)

weierophinney added a commit to weierophinney/zend-inputfilter that referenced this pull request Feb 18, 2016
@weierophinney weierophinney merged commit 310470c into zendframework:develop Feb 18, 2016
weierophinney added a commit that referenced this pull request Feb 18, 2016
weierophinney added a commit that referenced this pull request Feb 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants