-
Notifications
You must be signed in to change notification settings - Fork 479
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
Fixed "ERROR: Using ${var} in strings is deprecated, use {$var} instead" and added logError and logErrorDetails variables on the Shutdown handler #351
base: main
Are you sure you want to change the base?
Conversation
Why "Tests / Tests PHP 8.1 (pull_request)" is failed? |
Why "Tests / Tests PHP 8.1 (pull_request)" is failed? Because of using more than one whitespace between the return type declaration and the variable in a function signature. That may look better, but is against the coding guidelines here… EDITed this post: The stuff I wrote before seems to be against the coding guidelines seems legitimate and isn't what's in question here. |
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.
Don't like this.. (not a team member, just looking at this) couldn't you just remove the ` parts instead of leaving the string?
I fixed the php 8.1 test issues but now I'm getting a new error from Coveralls, why? |
Because the new changes aren't covered in the unit tests...but I'm not sure if we should do it that way. If the error given to the shutdown handler is an error in the logger itself, we shouldn't give it to the logger. On the other hand, if it wasn't, we should, so I think we should make some try/catch around it. (Only my personal view, not a maintainer.) |
Fixed php error on view user action (src\Application\Actions\User\ViewUserAction.php):
ERROR: Using ${var} in strings is deprecated, use {$var} instead"