search

tboothman / imdbphp

PHP library for retrieving film and tv information from IMDb
251 stars 84 forks source link

PHP 5.6 support broken again #290

Closed paxter closed 1 year ago

paxter commented 1 year ago

Please stop official PHP 5.6 support, if you break support on every update.

public const MOVIE = Title::MOVIE;

There is no public in PHP 5.6. Affects multiple files.

jreklund commented 1 year ago

It was never my intension to break it this time either. I introduced PER Coding Style (to maintain consistent coding style), and missed PHP 7.0 and older support in that configuration. Our test suite can't run on anything older than PHP 7.1, I tried to run them in PHP 5.6 for what it's worth before releasing v7.4.0 today.

It have been patched in v7.4.2.

Versions not automatically tested:

Version automatically tested:

paxter commented 1 year ago

Hehe ok. Thanks for your answer and the update. :)

tboothman commented 1 year ago

I've changed the github action to do a rudimentary check against the older versions of php and test a few more of the new ones: https://github.com/tboothman/imdbphp/commit/40e5cc14a39b768fc162a6f6683aeb7b002077ff https://github.com/tboothman/imdbphp/actions/runs/3864274337 Travis used to do this but it stopped doing free builds a year or so ago

jreklund commented 1 year ago

@tboothman Great! Smart idea to have a check for instant crashes. I think it's time to upgrade our require-dev as well. PHPStan are logging multiple errors in PHP 8.1.

Maybe we should just use build for Supported Versions and update them to PHP 8.0? Last time I locked in PHP 7.4 in v7.3.1. We can still run PHP 7.4 with the latest PHPStan (if we increase the memory a little).

tboothman commented 1 year ago

For phpstan it only needs to work on one version as the results will always be the same. I'm on 8.1 and 7.4.

For the tests it'd be nice if it ran on multiple versions because they might work differently and while we can keep older versions working I think it's worth it. I've got this running on my server/NAS on PHP 7.4 and that seems to be the latest version it has so I'd be reluctant to stop testing a version I use. The newer versions of php don't have anything that'd make the library any better (afaik) (without making it backwards incompatible) so I don't think it's too much of a chore supporting older versions.

The version of PHPUnit we're using doesn't run on <7.4, and I don't think there was one that works for all the versions of php we're using. A version that works on as wide a range as possible (including the latest) would be best

jreklund commented 1 year ago

I have now updated our dev requirements so we can use a newer version of phpstan and it found three problems in the progress. Newer versions of PHP brings better syntax, we don't require any new functionality. I have set PHP 7.4 as build target so we don't get any package recommendation beyond that version.

Syntax check:

Test:

Build: