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.

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

Upgrade to PHP 7.1+ #271

Merged
merged 2 commits into from Jan 15, 2019
Merged

Upgrade to PHP 7.1+ #271

merged 2 commits into from Jan 15, 2019

Conversation

GabrielAnca
Copy link
Contributor

Why?

PHP 5.6 and 7.0 are no longer supported. We should only support active versions (7.1, 7.2 and 7.3 at the time of writing)

How?

This PR drops the support for PHP < 7.1 and splits the circle tests to ensure it works in all of those versions.

PHPUnit 4 is both unsupported and not compatible with PHP >=7.2. PHPUnit 7 is compatible with php 7.1 to 7.3, so I'm upgrading to that version.

@GabrielAnca GabrielAnca force-pushed the ga/php-7 branch 3 times, most recently from 5f814bf to 1fd3e3f Compare January 7, 2019 13:57
@@ -7,8 +7,7 @@
convertNoticesToExceptions="true"
convertWarningsToExceptions="true"
processIsolation="false"
stopOnFailure="false"
syntaxCheck="false">
Copy link
Contributor

Choose a reason for hiding this comment

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

why have we removed this syntax check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That argument has been removed from the last versions of PHPUnit, see the answer by PHPUnit author here. I googled all I could and I can't even find what exactly that was doing 😕

SeanHealy33
SeanHealy33 previously approved these changes Jan 9, 2019
@GabrielAnca GabrielAnca changed the title Upgrade to PHP 7 Upgrade to PHP 7.1+ Jan 14, 2019
DavidGarciaCat
DavidGarciaCat previously approved these changes Jan 14, 2019
Copy link

@DavidGarciaCat DavidGarciaCat left a comment

Choose a reason for hiding this comment

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

Hey @GabrielAnca

The PR makes sense to me, just a minor amend on the README file.

Cheers,

README.md Outdated Show resolved Hide resolved
Co-Authored-By: GabrielAnca <GabrielAnca@users.noreply.github.com>
Copy link
Member

@choran choran left a comment

Choose a reason for hiding this comment

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

👍

@GabrielAnca GabrielAnca merged commit ea68849 into master Jan 15, 2019
@GabrielAnca GabrielAnca deleted the ga/php-7 branch January 15, 2019 11:09
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.

None yet

4 participants