jmikola / geojson

GeoJSON implementation for PHP
MIT License
295 stars 47 forks source link

Drop support for PHP <7.4, update PHPUnit to ~9.5, migrate to GitHub Actions #31

Closed jeromegamez closed 2 years ago

jeromegamez commented 2 years ago

Hey there πŸ‘‹,

while working on a PHP 8.1 project, I noticed the following deprecation message

PHP Deprecated: Return type of GeoJson\GeoJson::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /path/to/code/vendor/jmikola/geojson/src/GeoJson/GeoJson.php on line 64

the reason probably being the JsonSerializable stub, and I thought this might be an excellent opportunity to propose a 1.1 release of this library.

I tried to keep the changes as atomic as possible and as minimal as needed, just enough to support the updated dependencies, which are:

This PR supersedes #27 and #29.

I'd greatly appreciate it if you could let me know if you could see these changes in a 1.1 release πŸ™ . I have more improvements in mind (and already prepared πŸ˜…), e.g., updated usage of language features and type checks, but I didn't want to blow this PR more than necessary.

Thank you! :octocat:

jeromegamez commented 2 years ago

I just added a commit that prevents the upload of the code coverage report when not in this repository.

It's possible that you'd have to activate the actions on https://github.com/jmikola/geojson/settings/actions so that they can run on this PR - if you want to have a look beforehand, here is a (now) successful run on my fork: beste/jmikola-geojson/actions/runs/1721431686

jmikola commented 2 years ago

I took a look at https://github.com/jmikola/geojson/settings/actions and everything seems correct. According to https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks there should be a UI on the PR itself to approve the workflow but it's not visible. I assume that may be because we don't already have a workflow configuration and this PR is introducing it.

I'll just run the tests locally this evening and trust that the workflow will become active after merging. And then this shouldn't be a problem for future PRs.

jmikola commented 2 years ago

I reverted the breaking commit from https://github.com/jmikola/geojson/pull/24 in master and then created a v1.0 branch. Thankfully, that code was never released.

This PR has now been manually merged to master with some follow-up changes to composer.json and the GitHub workflow. Thanks!