[Console] Sync ConsoleLogger::interpolate with the one in HttpKernel #24527
Conversation
|
Wouldn't it be better to share the code instead of copy/pasting it? |
|
Where would stay this code? There is no dependency in common between |
|
@sroze these are two independent components, so sharing is not possible without creating new deps, which we don't want to. |
| $replace[sprintf('{%s}', $key)] = $val; | ||
| if (null === $val || is_scalar($val) || (\is_object($val) && method_exists($val, '__toString'))) { | ||
| $replacements["{{$key}}"] = $val; | ||
| } elseif ($val instanceof \DateTime || $val instanceof \DateTimeInterface) { |
There was a problem hiding this comment.
note to mergers: first condition should be removed starting from 3.3
There was a problem hiding this comment.
@dunglas can you please rebase+retarget, and take this comment into account meanwhile?
|
This seems to break the appveyor build :/ (ran twice, same breakage with no output) |
|
Not a bug fix, should be merged in 3.4 instead. |
9e6ace0 to
8fcbc55
Compare
|
Status: Needs Review |
|
Thank you @dunglas. |
…n HttpKernel (dunglas) This PR was merged into the 3.4 branch. Discussion ---------- [Console] Sync ConsoleLogger::interpolate with the one in HttpKernel | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget to update UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Adapted from: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/Log/Logger.php#L92 Better performance and datetime support (maybe should it be merged in a newer version, but I've targeted 2.7 to prevent future merge conflicts). Commits ------- 8fcbc55 [Console] Sync ConsoleLogger::interpolate with the one in HttpKernel
Adapted from: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/Log/Logger.php#L92
Better performance and datetime support (maybe should it be merged in a newer version, but I've targeted 2.7 to prevent future merge conflicts).