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

[Console] Sync ConsoleLogger::interpolate with the one in HttpKernel #24527

Merged
merged 1 commit into from Oct 15, 2017

Conversation

Projects
None yet
6 participants
@dunglas
Member

dunglas commented Oct 12, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
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).

@sroze

This comment has been minimized.

Show comment
Hide comment
@sroze

sroze Oct 12, 2017

Member

Wouldn't it be better to share the code instead of copy/pasting it?

Member

sroze commented Oct 12, 2017

Wouldn't it be better to share the code instead of copy/pasting it?

@dunglas

This comment has been minimized.

Show comment
Hide comment
@dunglas

dunglas Oct 12, 2017

Member

Where would stay this code? There is no dependency in common between console and http-kernel. And creating a new component just for this method looks overkill.

Member

dunglas commented Oct 12, 2017

Where would stay this code? There is no dependency in common between console and http-kernel. And creating a new component just for this method looks overkill.

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 12, 2017

Member

@sroze these are two independent components, so sharing is not possible without creating new deps, which we don't want to.

Member

nicolas-grekas commented Oct 12, 2017

@sroze these are two independent components, so sharing is not possible without creating new deps, which we don't want to.

@chalasr

This comment has been minimized.

Show comment
Hide comment
@chalasr

chalasr Oct 12, 2017

Member

This seems to break the appveyor build :/ (ran twice, same breakage with no output)

Member

chalasr commented Oct 12, 2017

This seems to break the appveyor build :/ (ran twice, same breakage with no output)

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 12, 2017

Member

Not a bug fix, should be merged in 3.4 instead.

Member

fabpot commented Oct 12, 2017

Not a bug fix, should be merged in 3.4 instead.

@nicolas-grekas nicolas-grekas modified the milestones: 2.7, 3.4 Oct 13, 2017

@dunglas dunglas changed the base branch from 2.7 to 3.4 Oct 15, 2017

@dunglas

This comment has been minimized.

Show comment
Hide comment
@dunglas

dunglas Oct 15, 2017

Member

Status: Needs Review

Member

dunglas commented Oct 15, 2017

Status: Needs Review

@chalasr

This comment has been minimized.

Show comment
Hide comment
@chalasr

chalasr Oct 15, 2017

Member

Thank you @dunglas.

Member

chalasr commented Oct 15, 2017

Thank you @dunglas.

@chalasr chalasr merged commit 8fcbc55 into symfony:3.4 Oct 15, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

chalasr added a commit that referenced this pull request Oct 15, 2017

minor #24527 [Console] Sync ConsoleLogger::interpolate with the one i…
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment