search

fxpio / composer-asset-plugin

NPM/Bower Dependency Manager for Composer
MIT License
892 stars 156 forks source link

fixed deprecated MultiConstraint class in VcsPackageFilter #157 #158

Closed nadar closed 8 years ago

nadar commented 9 years ago

What would be the best approach to guarantee backwards compatibility? I can not relay on Composer\Composer::Version as already mentioned in https://github.com/francoispluchino/composer-asset-plugin/pull/133#issuecomment-121978687

francoispluchino commented 9 years ago

Create a private method for build a new instance of MultiConstraint and check if Composer\Semver\Constraint\MultiConstraint class exist. if it's not the case, use Composer\Package\LinkConstraint\MultiConstraint class.

xabbuh commented 9 years ago

Looks like some tests need to be fixed too.

nadar commented 9 years ago

@francoispluchino I could now reproduce the failling test, but as a matter of complexity i could not figure out exactly how to fix:

phpunit failure:

Fxp\Composer\AssetPlugin\Tests\Repository\VcsPackageFilterTest::testFilterWithInstalledPackage with data set #39 (array(true, false), 'acme/foobar', 'v0.9.0', 'dev', '>=0.9@stable', '1.0.0', false)
Argument 3 passed to Composer\Package\Link::__construct() must implement interface Composer\Package\LinkConstraint\LinkConstraintInterface, instance of Composer\Semver\Constraint\MultiConstraint given, called in /var/www/composer-asset-plugin/Repository/VcsPackageFilter.php on line 324 and defined
"composer/composer": "dev-master#42bfe9c56adb555cc08e9ce2d32f6763ff75ae5d",
huebs commented 9 years ago

@nadar I originally made that change to .travis.yml which added a test configuration for that particular build of composer. The purpose was to check the backwards compatibility of the fix I had made in this pull request.

@francoispluchino It seems like this could be removed at this point as the build of composer it was checking is quite old.

francoispluchino commented 9 years ago

@huebs Yes, I thinks we can remove the backward compatibility now. I will release a new minor version after this PR.

francoispluchino commented 9 years ago

@nadar Can you remove travis tests (and backward compatibility of #133), and rebase your PR with only one commit, when your PR will be ready?

nadar commented 8 years ago

@francoispluchino Backward compatibility have been removed from tests, commits have been squashed.

nadar commented 8 years ago

@francoispluchino Should i remove those parts:

https://coveralls.io/builds/4079929/source?filename=Repository%2FVcsPackageFilter.php#L283 https://coveralls.io/builds/4079929/source?filename=Repository%2FVcsPackageFilter.php#L305

As they are removed from the tests:

https://github.com/francoispluchino/composer-asset-plugin/pull/158/files#diff-a2cfc999f7e7ca559322c6fe3a4df4caL555

or do you want to keep this backwards compatibility in the code (but not in the tests)?

francoispluchino commented 8 years ago

Remove backwards compatibility in code and tests.