Php8.4 support added #43
Conversation
|
@froschdesign Kindly review the PR once and do needful. |
|
@gsteel, kindly review this PR once and merge it, if possible. |
src/AutoDiscover.php
Outdated
| */ | ||
| public function handle() | ||
| public function handle($responseHandler = null) |
There was a problem hiding this comment.
@alexmerlin
The class is not final therefore this cannot be correct.
There was a problem hiding this comment.
@froschdesign We have made the class final. Please review once.
There was a problem hiding this comment.
@Atul-glo35265 make class final is bc break, if it required, uses func_get_args() may can handle that, see https://3v4l.org/8na14
There was a problem hiding this comment.
@Atul-glo35265
Changing the method signature and making a class final represent a break in backwards compatibility.
The question that arises here is why was the method changed at all? Only for tests?
There was a problem hiding this comment.
@froschdesign There were 3 errors in QA Checks (PHPUnit [8.4, lowest] failure. We made changes in these respective methods to fix the issue.
I'll try to implement if we can achieve the same result by implementing func_get_args() as suggested by samsonasik
There was a problem hiding this comment.
@froschdesign I've updated the version to 9.6.21 but the issue is still there.
Actually, the annotation @runinseparateprocess is causing the issue and throwing the exception. If we remove this annotation, issues related to Header already sent start coming.
I tried to upgrade PHPUnit to version ^10.0.0, but it seems we need to upgrade other dependencies as well.
There was a problem hiding this comment.
I've updated the version to 9.6.21 but the issue is still there.
The version of PHPUnit must be changed in the composer.json file!
There was a problem hiding this comment.
@froschdesign I have updated it in composer.json file. Please check and suggest if any other changes are required.
There was a problem hiding this comment.
Bump also all other packages in require-dev section.
There was a problem hiding this comment.
If bumping does not work, you can upgrade them manually. I just tested, they can be upgraded.
Atul-glo35265
force-pushed
the
php8.4_support
branch
from
3e78367
to
6ed98b4
Compare
|
@Atul-glo35265 "conflict": {
"amphp/amp":"<2.6.4"
} |
Signed-off-by: Atul-glo35265 <[email protected]>
Atul-glo35265
force-pushed
the
php8.4_support
branch
from
aea3c2a
to
6c555bb
Compare
Thank you @froschdesign. Please review once and do the needful. |
Description