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

Add support for PHP 7.2 #242

Merged
merged 6 commits into from
Apr 17, 2018
Merged

Conversation

thexpand
Copy link
Contributor

@thexpand thexpand commented Feb 5, 2018

No description provided.

@chris-kruining
Copy link
Contributor

just a sec, I'll submit my fixes as a PR

@chris-kruining
Copy link
Contributor

sorry, didn't notice this is a PR... whoops

@samsonasik
Copy link
Contributor

php 7.1 and 7.2 need to be registered to .travis.yml

@thexpand
Copy link
Contributor Author

@samsonasik Where can I take a look how it's done, so that I can complete this?

@samsonasik
Copy link
Contributor

there is .travis.yml file which the php versions can be registered, for example https://github.com/zendframework/ZendDeveloperTools/blob/master/.travis.yml#L26-L35 , you can add php 7.1 and php 7.2 there after php 7

.travis.yml Outdated
- php: 7.1
env:
- DEPS=locked
- CS_CHECK=true
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to run cs-check on every PHP version - please remove it here and also on PHP 7.2.

Copy link
Member

@michalbundyra michalbundyra left a comment

Choose a reason for hiding this comment

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

LGTM 👍

We just need update travis configuration/lock file, because it failing on PHP 5.6 and 7.2 locked.

@thexpand
Copy link
Contributor Author

@webimpress Is there anything else I can do to help?

@michalbundyra
Copy link
Member

@thexpand It would be nice if you can update the .travis.yml configuration. So basicaly we need define LEGACY_DEPS (see for example: https://github.com/zendframework/zend-servicemanager/blob/master/.travis.yml#L22) - I can't say what it should be in LEGACY_DEPS list in case of this repo - you need to figure it out based on current build logs.
Then we need update these legacy deps, as it is here:
https://github.com/zendframework/zend-servicemanager/blob/master/.travis.yml#L62-L63

Then we should be able to update composer.lock to the latest version (by running composer update) and all build should be fine.

These provide full support for PHP 7. Doing so required updating test
cases as these versions offer namespaced versions of their classes.
@@ -41,7 +41,7 @@
"zendframework/zend-view": "^2.6"
},
"require-dev": {
"phpunit/phpunit": "^4.8 || ^5.0",
"phpunit/phpunit": "^5.7.25 || ^6.4.4",
Copy link
Member

Choose a reason for hiding this comment

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

We can have also || ^7.1

@weierophinney weierophinney merged commit b90661a into zendframework:master Apr 17, 2018
weierophinney added a commit that referenced this pull request Apr 17, 2018
weierophinney added a commit that referenced this pull request Apr 17, 2018
weierophinney added a commit that referenced this pull request Apr 17, 2018
@weierophinney
Copy link
Member

Thanks, @thexpand. I have updated the Travis configuration, and also upped the minimum supported
version of PHPUnit so that it will work correctly with PHP 7.2; last tests passed all versions!

@thexpand thexpand deleted the thexpand-php72 branch April 18, 2018 06:56
@thexpand
Copy link
Contributor Author

@weierophinney Thanks for helping out. I was extremely busy lately and moreover I was not quite sure how to update the Travis file. Could you explain to me your changes?

@weierophinney
Copy link
Member

@thexpand The approach is as follows:

  • COMPOSER_ARGS only contains --no-interaction
  • Individual locked environments add a LEGACY_DEPS env variable with specific dependencies that we know need different versions than are in the lock file.
  • In the install target:
    • We do an initial composer install that also includes the --ignore-platform-reqs flag in order to install from the lock file.
    • We then check for a non-empty LEGACY_DEPS env variable, and, if found, run composer update --with-dependencies $LEGACY_DEPS; this installs the platform-specific versions of the dependencies listed, ensuring that there are no PHP version conflicts.
    • From there, we check for lowest/latest envs, and do composer updates accordingly
    • At the end, we cal stty cols 120 && composer show to list what dependencies were installed; the stty bit is to work around how composer shows the dependency table in non-TTY environments.

The only other change I made was to update PHPUnit to allow using versions 5.7+, 6, or 7 (and no longer use 4), as PHPUnit 4 and early 5.X series do not run under PHP 7.1 and up. This also meant some minor changes to the test suite.

@thexpand
Copy link
Contributor Author

Detailed explanation as always 😉 Thank you once again, @weierophinney

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants