Add PHP 8+ support, new test workflows #14
Conversation
ababkov changed the title 
composer.json Outdated
| "rexlabs/utility-belt": "^3.0" | ||
| "php" : "^8.0", | ||
| "rexlabs/utility-belt": "dev-feat/github-tests", | ||
| "dms/phpunit-arraysubset-asserts": "^0.5.0" |
There was a problem hiding this comment.
Should this package be included under require-dev if it's only needed to support the tests?
src/ArrayObject.php Outdated
| * @throws \Rexlabs\ArrayObject\Exceptions\InvalidOffsetException | ||
| */ | ||
| public function offsetGet($offset) | ||
| public function offsetGet($offset): mixed |
There was a problem hiding this comment.
I see that we are dropping support for php 7. I think it's because of this change as that's only supported from php 8. I am wondering if that is strictly required here.
Are we looking at just going with php 8 migration for wings without running in parallel with 7.4 like we did with the previous upgrade? I am not against it, just trying to understand that context for this change 🙂
There was a problem hiding this comment.
PHP 8 fatals or warns can't recall due to not matching the underlying interface return types.
mixed was only introduced in php 8.
If you remove this the tests pass in php 7 but fail in all versions of php 8.
If you add this, the tests pass in php 8 but fail in all versions of php 7.
You could maybe? do this as a two stage release:
- Release 1: PHP 7.4 without this change with tests only for 7.4 tag as 4.0.0
- Release 2: PHP 8 (drop PHP 7.4) with this change with tests only for 8 tag as 4.1.0
There was a problem hiding this comment.
I am happy for us to go all in with php 8. Thank you for the explanation 🙏
There was a problem hiding this comment.
Going to try for the two stage release.
There was a problem hiding this comment.
2 stage release not needed - #14 (comment)
ababkov force-pushed the feat/php-8 branch 9 times, most recently from e37c9a4 to 1b5afa0 Compare 
| * | ||
| * @return mixed | ||
| */ | ||
| #[\ReturnTypeWillChange] |
There was a problem hiding this comment.
@BlankaK found a way to avoid the php 7/8 issue that's caused by type hints in this case. There's a specific attribute that php 8 introduces to help deal with cross version compatibility issues. https://www.php.net/manual/en/class.returntypewillchange.php
ababkov changed the title 
Overview
Before merge
rexlabs/utility-beltto be4.0.0hereAfter Merge