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
Convert level/levelName to enums #1656
Conversation
@@ -30,11 +28,11 @@ | |||
'cast_spaces' => ['space' => 'single'], | |||
'header_comment' => ['header' => $header], | |||
'include' => true, | |||
'class_attributes_separation' => ['elements' => ['method']], | |||
'class_attributes_separation' => array('elements' => array('method' => 'one', 'trait_import' => 'none')), |
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.
why not keeping short arrays ?
Logger::ALERT => 'error', | ||
Logger::EMERGENCY => 'error', | ||
]; | ||
protected function toWildfireLevel(Level $level): string |
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.
why is it protected while it was private before ?
@@ -203,16 +203,12 @@ public function getSlackData(LogRecord $record): array | |||
*/ | |||
public function getAttachmentColor(int $level): string |
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.
shouldn't this get a Level as argument ?
src/Monolog/Level.php
Outdated
public function toPsrLogLevel(): string | ||
{ | ||
return match ($this) { | ||
self::Debug => 'debug', |
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.
why not using the LogLevel constants there ?
src/Monolog/LogRecord.php
Outdated
// avoid returning readonly props by ref as this is illegal | ||
$copy = $this->{$offset}; | ||
|
||
return $copy->value; |
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.
should you return the value of an enum by ref ?
src/Monolog/Logger.php
Outdated
{ | ||
$recordInitialized = count($this->processors) === 0; | ||
|
||
$record = new LogRecord( | ||
message: $message, | ||
context: $context, | ||
level: $level, | ||
level: $level instanceof Level ? $level : self::toMonologLevel($level), |
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.
toMonologLevel
already handles the case of instanceof Level
src/Monolog/Logger.php
Outdated
@@ -494,7 +474,7 @@ public function getExceptionHandler(): ?callable | |||
*/ | |||
public function log($level, string|\Stringable $message, array $context = []): void | |||
{ | |||
if (!is_int($level) && !is_string($level)) { | |||
if (!is_string($level) && !is_int($level) && !$level instanceof Level) { | |||
throw new \InvalidArgumentException('$level is expected to be a string or int'); |
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.
or a Level
Thanks @stof for the review 👍🏻 Will merge now then. |
Refs #1648
See Monolog\Level & Monolog\LevelName