Optionally allow specifying a log level map in the constructor. #1
Conversation
There was a problem hiding this comment.
The general approach of using a level map somewhere makes sense to me, because any methods that don't take an explicit level value (like debug() or hasDebugThatContains()) needs some way to know the values being used. I think my two biggest questions right now are:
- Is
hasRecordThatPasses()truly the best place to perform that translation with the mapping? (Should we do it when the record is logged instead? Somewhere else? I don't know) - Should we modify the
string $levelmethod params to accept ints too?
See my other thoughts in colinodell#2 (comment) - I'd love your feedback on them!
Given my comments in colinodell#2 (comment), I wonder if maybe we should just change `string $level
| $mappedLevel = $this->levelMap && array_key_exists($level, $this->levelMap) | ||
| ? $this->levelMap[$level] | ||
| : $level; |
There was a problem hiding this comment.
I think this could be simplified to $mappedLevel = $this->levelMap[$level] ?? $level;
| /** | ||
| * @param null|array<string, mixed> $levelMap | ||
| * Keys are LogLevel::*, values are anything that can be an array key. | ||
| */ | ||
| public function __construct(?array $levelMap = NULL) | ||
| { | ||
| $this->levelMap = $levelMap; | ||
| } |
There was a problem hiding this comment.
I'd be okay with making this non-nullable. We could also change string in the docblock to LogLevel::* so that phpstan and psalm can help ensure the right keys are set by callers.
See colinodell#2.