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

Add PHP 8.0 support #169

Merged
merged 7 commits into from
Jan 9, 2022
Merged

Add PHP 8.0 support #169

merged 7 commits into from
Jan 9, 2022

Conversation

lumnn
Copy link
Contributor

@lumnn lumnn commented Oct 5, 2021

This is absolute minimum for PHP 8.0 support.

My work here is only to get composer run test work without any warnings and deprecation messages.

I did not run the code in any other way than through tests, so I'm not 100% confident about everything working correctly, but I have to leave now, so I may give it a bit more attention tomorrow ;)

This relates to issue #167

@lumnn lumnn changed the title Add PHP 8.0 support Add PHP 8.0 support #167 Oct 5, 2021
@lumnn lumnn changed the title Add PHP 8.0 support #167 Add PHP 8.0 support Oct 5, 2021
@JoshStaff JoshStaff mentioned this pull request Dec 1, 2021
@thtg88
Copy link

thtg88 commented Jan 9, 2022

Hey all 👋 is there anything I can do to help this PR being merged? Would be great to be able to use this package with PHP 8 :)

@barryvdh
Copy link
Member

barryvdh commented Jan 9, 2022

Hmm was hoping to run the tests but not much happens.

@barryvdh barryvdh closed this Jan 9, 2022
@barryvdh barryvdh reopened this Jan 9, 2022
@barryvdh barryvdh merged commit 900ca00 into thephpleague:master Jan 9, 2022
@barryvdh barryvdh mentioned this pull request Jan 9, 2022
@thtg88
Copy link

thtg88 commented Jan 9, 2022

Hmm was hoping to run the tests but not much happens.

I think it might have been because the branch wasn't on the main repo and the owner had not enabled GitHub Actions on it 🤔

Thanks for merging!

@barryvdh
Copy link
Member

barryvdh commented Jan 9, 2022

Hmm I can't run it on 7.2 because of prophecy supports 7.3 and up. Removed it in #171

Any ideas for that? Otherwise it isn't the biggest problem in the world, but still.. We could just bump the PHP version to 7.3 I guess.

@barryvdh barryvdh mentioned this pull request Jan 9, 2022
@thtg88
Copy link

thtg88 commented Jan 9, 2022

Hmm I can't run it on 7.2 because of prophecy supports 7.3 and up. Removed it in #171

Any ideas for that? Otherwise it isn't the biggest problem in the world, but still.. We could just bump the PHP version to 7.3 I guess.

To solve the composer install requirements issue we could change (in composer.json) the Prophecy requirement to:

- "phpspec/prophecy-phpunit": "^2.0"
+ "phpspec/prophecy-phpunit": "^1.1|^2.0"

The problem is that the Prophecy API changed with 2.0 (as you'd expect with major versions), so the ProphecyTrait being used in tests is not available in 1.1. I think dropping PHP 7.2 might be the best option here 🤔

@lumnn
Copy link
Contributor Author

lumnn commented Jan 9, 2022

Just to clarify on PHP version, I believe I put 7.2 because this is what was in omnipay/tests. If it comes to me, I don't much care about 7.2, because it's unsupported for a year, therefore I believe it's quite reasonable to drop 7.2 version, which anyway is supported by currently latest release of this package.

@barryvdh
Copy link
Member

barryvdh commented Jan 9, 2022

Agreed. Bumped to 7.3 in 9ddf4fa. Bumped the branch alias to 3.3 for testing so I suggest we release 3.3.0 sometimes with support for 7.3 and up. We can always still release a new 3.2.x version in case of issues.

@barryvdh
Copy link
Member

barryvdh commented Jan 9, 2022

@judgej do you think we should release the commits before this PR as a new 3.2 version? And then tag this as 3.3?

@judgej
Copy link
Member

judgej commented Jan 9, 2022

Yes, that seems the right thing to do. I have personally not had time to test these changes, but if people are using them in production, even if just their own branches, then go for it. I have some sites I need to uograde anyway, that will become very urgent soon, so will be available then for any tweaks or addiutional documentation.

So I understand, v3.2 would be the last PHP 7.2 version, then v7.3 v3.3 would be PHP 7.3+?

@thtg88
Copy link

thtg88 commented Jan 9, 2022

So I understand, v3.2 would be the last PHP 7.2 version, then v7.3 would be PHP 7.3+?

(As i understand it) v3.3 would be PHP 7.3+

@barryvdh
Copy link
Member

barryvdh commented Jan 9, 2022

Yeah

@thtg88
Copy link

thtg88 commented Jan 13, 2022

I think I'll be able to start testing this (off of master) tomorrow or first thing next week. I'll let you know if I hit any issue with PHP 8.0 👍

@thtg88
Copy link

thtg88 commented Jan 26, 2022

I can confirm that at least the form methods work as expected with PHP 8.0 - is there anything I can do to help tag an official release? Or would you prefer to have some more testers e.g. for the other integration methods (server, direct)? :)

@jamieburchell
Copy link
Contributor

jamieburchell commented Jan 30, 2022

Also volunteering if required to test against the Server integration

@jamieburchell
Copy link
Contributor

@barryvdh @judgej Is there anything that we can do to get a new tagged release with PHP 8 support?

@benjam-es
Copy link
Contributor

@jamieburchell if you're happy with the update, could you use the specific branch / commit reference within your composer.json to enable use now?

Obviously not ideal, but could be a temporary solution until they manage a release.

@benjam-es
Copy link
Contributor

@jamieburchell also see this: #173 could be worth a test?

@jamieburchell
Copy link
Contributor

jamieburchell commented Feb 11, 2022

@jamieburchell if you're happy with the update, could you use the specific branch / commit reference within your composer.json to enable use now?

Obviously not ideal, but could be a temporary solution until they manage a release.

@benjam-es Sure, but I have no idea how stable that is or how much compatibility there is with PHP 8.x and I'd rather not be pointing production applications at random commits.

I appreciate that maintainers have jobs and lives, but this repo hasn't had a release in nearly 3 years and I'm at the stage where I need to decide if I am going to fork this repo and take it on, or if there is something I can do to help this repo get a stable release.

I will be doing some testing against the master branch, but I'm not familiar enough with the project or its development to be confident that it's doing what it should be.

@benjam-es
Copy link
Contributor

For now, to help out, I think if you and other users pointed a testing site to the master branch, and attempted to do some testing to ensure it works as you want it to, and hopefully do some testing to see if it works for protocol 4 given the latest merged pull request.

I believe the tests which are also run on CI, give the proof that it they work for php 8.0, so you would likely have to review all code change to ensure you are happy before pointing to a commit/branch.

In my case, I forked it, and I have my fork on a private satis so I use that instead until there are official releases.

@jamieburchell
Copy link
Contributor

For now, to help out, I think if you and other users pointed a testing site to the master branch, and attempted to do some testing to ensure it works as you want it to, and hopefully do some testing to see if it works for protocol 4 given the latest merged pull request.

I pointed a PHP 7.4 testing site at master (protocol v4) and basic purchase transactions worked without issue. I also tested the abort from a client and from a timeout.

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.

6 participants