Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dev dependencies should include the compatible PHPunit version #34

Merged
merged 3 commits into from
Aug 21, 2016
Merged

Dev dependencies should include the compatible PHPunit version #34

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9455e77
Dev dependencies should include the compatible PHPunit version
curry684 Aug 18, 2016
d61c405
Degraded PHPunit and code-coverage versions for PHP 5.5 compat
curry684 Aug 18, 2016
c23e290
Removed phpunit/code-coverage dependency and upped PHP to 5.5.9
curry684 Aug 20, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
},
"require-dev": {
"league/oauth2-facebook": "^1.1",
"symfony/security-guard": "^2.8|^3.0"
"symfony/security-guard": "^2.8|^3.0",
"phpunit/phpunit": "^4.8",
Copy link
Member

Choose a reason for hiding this comment

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

I think we should allow 5.x version of PHPUnit in this case too with ^4.8|^5.0, right?

Copy link
Contributor Author

@curry684 curry684 Aug 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It would also need finetuning of the php-code-coverage since they're intertwined (phpunit 5 requires code coverage 3 or 4), but yeah it could work. In general though I myself favor just picking a single major to avoid having differences between developers. I would just postpone the 5.x upgrade until PHP 5.5 is dropped in this project itself.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with using just one major, as this - because being a dev dependency, this won't cause any problems with end-user projects, correct (i.e. an end-user could still install phpunit/phpunit version 5 in their project, without this conflicting)?

And, is there a precedence for including php-code-coverage as a dependency in other OS projects? I'm just not familiar with this - and mostly, I want to follow what's most expected/standard.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recommend sticking to the single major for the universally reproducible results. As dev requirements are installed per project this means it's fine if you have 3 projects on the same machine depending on PHPunit ^4.6, 5.1.* and ^5.2 - each will get their own copy.

As dev requirements themselves are never propagated to external projects you can in practice do whatever the hell you want with them, it's your project 😄 php-code-coverage itself has been installed 25.5 million times so I'd think it enough precedent 😉

I'll pick this up along with #36, do expect a freakish PR coming up from that with all files modified hehe.

Copy link
Member

Choose a reason for hiding this comment

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

My question is why do we really need phpunit/php-code-coverage in this project? I agree with PHPUnit, we use it for tests, but we don't use php-code-coverage here. As for me adding a dependency which we don't use in a project just in case is a bad idea which leads to overhead and even could confuse other devs.

"phpunit/php-code-coverage": "^2.2"
},
"suggest": {
"symfony/security-guard": "For integration with Symfony's Guard Security layer"
Expand Down