-
-
Notifications
You must be signed in to change notification settings - Fork 365
Backport of PHP 8.5 patch to 5.x.x #864
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
base: 5.x.x
Are you sure you want to change the base?
Conversation
ci: Add PHP 8.5 to pipeline, ignoring dependencies and as experimental (jsonrainbow#842) This PR will - resolve deprecations for: - Reflection*::setAccessible - curl_close Fixes jsonrainbow#798 Fixes jsonrainbow#863
|
@reedy thanks for the PR and I’m willingly get this merged into 5.x.x. This might take some while as I’m enjoying the holidays atm. Looks like the CI pipeline indeed doesn’t work so I’ll have to jump on that before we merge things. Do you already found something or have a idea on why it isn’t running them? |
|
I'm pretty sure it's because there's no
The easiest answer is probably copying a version of the relevant CI files you want to run back into that branch, and it'll probably work fine... I guess that should be easily proved, will do a patch. No rush on my part, I'm just a little bored, and upgrading an ancient version of composer (while running on PHP 8.5) locally noticed another PHP 8.5 cleanup that needed doing in MediaWiki and various vendored libs, such as this one :) |
Remove old .travis.yml Update .gitattributes cf https://github.com/jsonrainbow/json-schema/tree/2911a17/.github/workflows
|
It starts becoming the eternal problem of how much of this we want to actually work... 2eb0def probably should be backported, but phpunit 8.5 only works on PHP >= 7.2 - https://packagist.org/packages/phpunit/phpunit#8.5.50 |
|
@reedy I’m back from holiday season. I’ll see what we could do for this PR somewhere this week. First approach would be to see how much should be solved. The PHP version support is very much tied against composer, so bumping PHP minimum level is not feasible. Will keep you updated. |
|
@reedy I've been diving into this and indeed this is becoming a true down the rabbit hole experience.
This way we keep the change to a minimal set and we trust on the wrapping as it is part of the main branch and shows it's working there. I also want to strongly encourage to migrate to version 6 of Json Schema as that has a decent CI pipeline for PHP >= 8.0 versions. |
Yeah, it's on our long todo list. I know it's not what's happening here, but I do find it funny when libraries have version constraints that suggest that they're supported on a version, and then won't do anything with bug reports... Don't put so wide constraints on then ;) And sure, they're only warnings, but when you're developing against those PHP versions, and their code is integrated in your app/stack... Anyway. Yeah, I'm not expecting anyone puts massive amounts of work into these things, it's definitely a best effort type of thing. Supporting wide ranges of PHP versions gets harder when phpunit and other things introduce a lot of changes in their major versions, and have narrower support versions. Anything is an improvement. Some CI is better than no CI. Ideally keeping some amount of PHP 8 CI would be better.
^ This could happen, but wouldn't necessarily need to change the supported PHP versions, just probably wouldn't run CI on 7.1 One would hope people aren't still running 7.1 in prod, being EOL for over 6 years, but you never know. 2201471 bumped the PHP requirement to 7.1 from 5.3 in a minor release. |
ci: Add PHP 8.5 to pipeline, ignoring dependencies and as experimental (#842)
This PR will
Fixes #798
Fixes #863