search

stecman / symfony-console-completion

Automatic tab-key completion for Symfony console application options, arguments and parameters
MIT License
420 stars 26 forks source link

Remove PHPUnit dev dependency #15

Closed aik099 closed 9 years ago

aik099 commented 9 years ago

Most of people have latest PHPUnit installed globally in one way or another. I suggest removing it from composer.json.

stecman commented 9 years ago

I agree that it's not entirely necessary to have, but I do find it useful to have the PHPUnit code within the project directory to get completions for PHPUnit classes/methods. I'm inclined to leave it in unless there's a good reason to remove it.

aik099 commented 9 years ago

If you have PHPUnit installed globally (e.g. via Composer), then you can (at least in PhpStorm):

  1. add the ~/.composer/vendor/autoload.php so that PhpStorm will detect it an can run tests
  2. add ~/.composer/vendor/phpunit/phpunit to the included paths to get auto-complete
duncan3dc commented 9 years ago

phpunit is a dev requirement for this project, so it's 100% correct that it is listed under require-dev, that's precisely what this feature is for

aik099 commented 9 years ago

I'll just put a reference to a discussion right about this subject here: https://github.com/hamcrest/hamcrest-php/pull/20

In general you're right, but for PHPUnit specifically that is not the case (see above discussion).

duncan3dc commented 9 years ago

Sorry, but I don't see how that discussion reaches consensus.

phpunit is required for the development of this package, so composer should look after this for you, it should not rely on you having globally installed phpunit, and hope that it is a compatible version

aik099 commented 9 years ago

Sorry, but I don't see how that discussion reaches consensus.

You're right. I bet PHPUnit is required for testing most of libraries. But how much of libraries have it as dev dependency? It would be really interesting to see the statistics.

For example if you have PHPUnit installed globally in one way or another and run tests by typing phpunit instead of vendor/bin/phpunit, then having yet another PHPUnit present under vendor folder just doesn't sound right.

Also if you want centrally update PHPUnit installation used to run tests (if PHPUnit isn't installed globally for example), then you need to individually do composer update for each library that has own copy of PHPUnit.

If decision is made to keep PHPUnit as dev dependency, then at least please relax version constrain to allow PHPUnit 4.x as well.

stecman commented 9 years ago

I'm closing this for now as I see it as a non-issue. I've updated the PHPUnit version constraint to 4.x, but I won't probably won't remove the dev dependency unless it's actually causing a problem for someone. Interesting debate though.

aik099 commented 9 years ago

Thanks.