Skip to content

Conversation

@dunglas
Copy link
Member

@dunglas dunglas commented Oct 13, 2016

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

@dunglas dunglas merged commit b7ffcd8 into api-platform:master Oct 13, 2016
@dunglas dunglas deleted the php71 branch October 13, 2016 10:27
@teohhanhui
Copy link
Contributor

But why would PHP 7.1 be used with the lowest dependencies? Shouldn't we add another matrix instead?

@dunglas
Copy link
Member Author

dunglas commented Oct 13, 2016

@teohhanhui it's a good idea to test the lowest version of dependencies with the highest version of the runtime. It maximises the probability to encounter errors because old versions of libraries are most likely the ones not supporting the last version of PHP.

We can add another line to the matrix but it will slow down the build for no real benefit (IMO). Does it worth it?

@teohhanhui
Copy link
Contributor

It maximises the probability to encounter errors because old versions of libraries are most likely the ones not supporting the last version of PHP.

It maximises the probability of other libraries messing up our tests, and that's not our intention. IMO The lowest dependencies test should use the lowest PHP version (if for no other reason than being realistic).

@dunglas
Copy link
Member Author

dunglas commented Oct 13, 2016

It's our responsibility to set correctly the min versions to use. Symfony (and most projects I follow) uses the same strategy.

magarzon pushed a commit to magarzon/core that referenced this pull request Feb 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants