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

[Travis] Test on PHP 7.1 #574

Closed
wants to merge 1 commit into from
Closed

[Travis] Test on PHP 7.1 #574

wants to merge 1 commit into from

Conversation

duncan3dc
Copy link
Member

No description provided.

@coveralls
Copy link

coveralls commented Oct 16, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 0c40344 on duncan3dc:patch-1 into a1ad0d0 on thephpleague:master.

shadowhand added a commit that referenced this pull request Oct 17, 2016
Ensures compatibility with PHP 7.1.

Refs #574
@shadowhand shadowhand mentioned this pull request Oct 17, 2016
@shadowhand
Copy link
Member

Looks like RandomLib 1.1 has some issues with PHP 7.1 that are fixed in version 1.2. I've created #575 to address this.

shadowhand added a commit that referenced this pull request Oct 17, 2016
Ensures compatibility with PHP 7.1.

Refs #574
@shadowhand
Copy link
Member

Please rebase this branch to master so that the tests run again.

@duncan3dc
Copy link
Member Author

Done 👍

@coveralls
Copy link

coveralls commented Oct 17, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 9c39b7d on duncan3dc:patch-1 into 6d62806 on thephpleague:master.

@shadowhand
Copy link
Member

shadowhand commented Oct 17, 2016

Looks like there are still errors when mcrypt is enabled, as per ircmaxell/RandomLib#55. Needs to be fixed upstream.

@GwendolenLynch
Copy link

@shadowhand FWIW due to PHP 7.1, in the Bolt project we've decided to replace RandomLib with polyfills for PHP 5.x as a result of the breakage/noise coming from this change, and Scott seems pretty determined to get the mcrypt stuff out a.s.a.p.

The hard(er) part for this project is the BC due to getter/setters in the abstract class, but thought it worth mentioning.

@shadowhand
Copy link
Member

Honestly, I think this good impetus for starting a version 3.x branch that is 7+ only. That would allow us to use type hints and drop RandomLib entirely.

@GwendolenLynch
Copy link

@shadowhand: No objections from us, but we (and a lot of projects) do need to support SF 2.8 (and 3.x from Q2/Q3 2017), which locks us at 5.5.9 for the foreseeable future.

So if needed, please see a ✋ raised from me if you eventually need someone to help maintaining/backporting to whatever the legacy branch becomes.

@ramsey
Copy link
Contributor

ramsey commented Nov 1, 2016

What's the status of this PR? Is more development being done to attempt to fix the failures on 7.1, or should I close this for now?

@shadowhand
Copy link
Member

@ramsey it should stay open. I will have time to dig in further this week.

@shadowhand
Copy link
Member

I'm going to address this by starting a 3.x branch that makes a few changes:

  • requires PHP 5.6
  • switches RandomLib for random_compat
  • updates to newest PHPUnit and drops Mockery

This will bring our dependencies up to latest releases and give us a solid base for starting a 4.x branch that drops PHP 5.x support.

@GwendolenLynch
Copy link

@shadowhand could we request that PHP 5.5.9 be the base PHP version for 3.x instead? As those of us producing products based on Symfony have that as a requirement.

At the end of the day, I don't think it would be too a huge pain for us, but it is often hard to explain upstream decisions to people and have them not respond with "Why don't you just use something else?"

@shadowhand
Copy link
Member

@GawainLynch what would be the purpose of that? PHP 5.5 is EOL and no longer receiving patches.

If you're locked into PHP 5.5, you can continue to use the 2.x series. The 3.x series will not change functionality, it will simply drop RandomLib for random_bytes (via random_compat).

@GwendolenLynch
Copy link

@shadowhand Supporting Symfony stack though to 4's release … but yeah, if it isn't good for you, no great stress. Just figured it couldn't hurt to ask, as RandomLib + PHP 7.1 is kinda painful for us right now too, we're in the position where we can extract it from Bolt's core use, but for our OAuth use-case it might still bite us, TBH haven't looked that hard yet as, well, RandomLib. 😆

@Art4
Copy link

Art4 commented Nov 3, 2016

@shadowhand

I'm going to address this by starting a 3.x branch...

Why to start with a 3.x branch and not with 2.x?

@shadowhand
Copy link
Member

shadowhand commented Nov 3, 2016

@Art4 replacing RandomLib results in API changes, since we will no longer have the getRandomFactory and setRandomFactory methods. Since we follow semver any API changes require a new major version.

@Art4
Copy link

Art4 commented Nov 3, 2016

I understand, but the last version is 1.4.2 (what I assume is calling the 1.x branch). Why do you want to skip the 2.x branch? Am I missing something?

@shadowhand
Copy link
Member

Oops, for whatever reason I thought we were on 2.x already. Thanks for pointing that out!

@Art4
Copy link

Art4 commented Nov 3, 2016

You're welcome 😄

shadowhand added a commit that referenced this pull request Nov 3, 2016
RandomLib depends on mcrypt, which is deprecated in PHP 7.1. The new
random functions that are present in PHP 7 are more than sufficient for
our needs and a compat package is available for PHP 5.x.

Refs #574
shadowhand added a commit that referenced this pull request Nov 3, 2016
RandomLib depends on mcrypt, which is deprecated in PHP 7.1. The new
random functions that are present in PHP 7 are more than sufficient for
our needs and a compat package is available for PHP 5.x.

Refs #574
@shadowhand shadowhand added this to the v2.0 milestone Nov 3, 2016
@shadowhand shadowhand added this to the v2.0 milestone Nov 3, 2016
shadowhand added a commit that referenced this pull request Nov 3, 2016
RandomLib depends on mcrypt, which is deprecated in PHP 7.1. The new
random functions that are present in PHP 7 are more than sufficient for
our needs and a compat package is available for PHP 5.x.

Also enables testing on PHP 7.1.

Refs #574
@shadowhand
Copy link
Member

Fixed by #583.

@shadowhand shadowhand closed this Nov 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants