Set minimum PHP to 7.4 #220
Conversation
Codecov Report
@@ Coverage Diff @@
## master #220 +/- ##
============================================
- Coverage 96.89% 96.68% -0.21%
Complexity 116 116
============================================
Files 13 13
Lines 483 453 -30
============================================
- Hits 468 438 -30
Misses 15 15
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
phil-davis
force-pushed
the
minimum-php-7.4
branch
from
5262714 to
1fbd85d
Compare
| @@ -50,7 +52,7 @@ public function __construct($value = null) | |||
| * | |||
| * If you are opening new elements, you must also close them again. | |||
| */ | |||
| public function xmlSerialize(Xml\Writer $writer) | |||
| public function xmlSerialize(Xml\Writer $writer): void | |||
There was a problem hiding this comment.
IMO this sort of change is what forces us to make a major version bump to v3 - anything that currently has Base.php as its parent will notice this change. xmlSerialize "suddenly" has a return type restricted to void. A child that re-implements xmlSerialize is likely to not be specifying any return type. PHP will start complaining when it notices that the child "specifies" a wider return type than the parent, violating LSP.
The child will have to add : void - so we can't release this as a 2.x version, because then composer might give the new version to projects automagically before thir code is ready.
There was a problem hiding this comment.
Agree.
Alternatively do the PR on 2.x with all changes but these and do the BC breaking stuff separately
There was a problem hiding this comment.
Alternatively do the PR on 2.x with all changes but these and do the BC breaking stuff separately
IMO the other parameter type changes... are "not that exciting" and mostly just in the test code. So there is not really much value for current users that are on PHP 7.1 7.2 7.3 to get those, they don't fix anything real.
| @@ -50,7 +52,7 @@ public function __construct($value = null) | |||
| * | |||
| * If you are opening new elements, you must also close them again. | |||
| */ | |||
| public function xmlSerialize(Xml\Writer $writer) | |||
| public function xmlSerialize(Xml\Writer $writer): void | |||
There was a problem hiding this comment.
Agree.
Alternatively do the PR on 2.x with all changes but these and do the BC breaking stuff separately
: voidto a child function...) in order to use this.This fixes issue #215 for this repo.
There are no other open PRs here, so I think we can do this one right away. If a serious bug is noticed in the old code, and we really need to make a patch release of 2.x then we can make a branch for that and backport the fix... But it is probably not worth bothering to set that up here - it might never be used.