-
Notifications
You must be signed in to change notification settings - Fork 718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add types for missing ones and bring psalm-baseline down to zero #1645
Conversation
d983354
to
ad0cf67
Compare
Signed-off-by: Simon L <[email protected]>
ad0cf67
to
8089ab8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think $args
should be array
: https://www.slimframework.com/docs/v4/objects/routing.html#route-callbacks
php/public/index.php
Outdated
@@ -140,7 +142,7 @@ | |||
}); | |||
|
|||
// Auth Redirector | |||
$app->get('/', function (\Psr\Http\Message\RequestInterface $request, \Psr\Http\Message\ResponseInterface $response, $args) use ($container) { | |||
$app->get('/', function (\Psr\Http\Message\RequestInterface $request, Response $response, mixed $args) use ($container) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$app->get('/', function (\Psr\Http\Message\RequestInterface $request, Response $response, mixed $args) use ($container) { | |
$app->get('/', function (Request $request, Response $response, mixed $args) use ($container) { |
consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, RequestInterface should be correct IIRC... (It is not a ServerRequest but a Client-Side request IIRC)...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name is misleading then, maybe import both as Request and ServerRequest to avoid the confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, saw this too late
Signed-off-by: Simon L <[email protected]>
Thanks for figuring this out! Fixed :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice even at errorLevel 2 🎉
Thanks a lot guys! :) |
Signed-off-by: Simon L [email protected]