EWPP-5162: Setup project for PHP 8.3 and port Notifications and Authentication classes #87
Conversation
There was a problem hiding this comment.
Lots of code quality improvments, I recomment we use the new https://github.com/openeuropa/code-review-library to be consistent
| * @param RequestInterface & Type\GetServiceTicket $getServiceTicketPart | ||
| * @return ResultInterface & Type\GetServiceTicketResponse |
There was a problem hiding this comment.
think we should indicate one or the other, usually its the interface, but if we need it to be the class then lets go with that
There was a problem hiding this comment.
This code is generated by the library, check the readme here.
| { | ||
| $new = clone $this; | ||
| $new->serviceTicket = $serviceTicket; | ||
|
|
||
| return $new; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This code is generated by the library, check the readme here.
| // // Add the ResultInterface to classes that match given regex. | ||
| // ->addRule( | ||
| // new Rules\TypenameMatchesRule( | ||
| // new Rules\AssembleRule(new Assembler\ResultAssembler()), | ||
| // '/Response$/' | ||
| // ) | ||
| // ) |
There was a problem hiding this comment.
I see this repo is full of commented out code, not for this PR but I suggest you make a ticket/issue where you uncomment this code and either move it to a separate branch, or add a flag to execute it only when true, but leaving code like this is a problem
There was a problem hiding this comment.
I will clean all of this in a follow up ticket, before closing the epic and releasing.
| */ | ||
| private $notificationType; | ||
| private $notificationType = null; |
There was a problem hiding this comment.
even without assigning null, the default value is null
There was a problem hiding this comment.
This code is generated by the library, check the readme here.
| @@ -12,7 +12,7 @@ | |||
| "cweagans/composer-patches": "^1 || ^2", | |||
| "php-http/logger-plugin": "^1", | |||
| "php-http/message": "~1.13", | |||
| "phpro/soap-client": "~2.4", | |||
| "phpro/soap-client": "~3.4", | |||
There was a problem hiding this comment.
As I understand php constraint should be updated as we are testing in CI only with 8.3 version.
| @@ -12,7 +12,7 @@ | |||
| "cweagans/composer-patches": "^1 || ^2", | |||
There was a problem hiding this comment.
Do we still need the ^1 version?
| "symfony/http-client": "~5.4 || ~6", | ||
| "symfony/serializer": "~5.4 || ~6", | ||
| "symfony/validator": "~5.4 || ~6", |
There was a problem hiding this comment.
Do we still need to keep the support of the ~5.4 versions for symfony-related packages?
There was a problem hiding this comment.
I'll remove this in the next ticket.
No description provided.