Skip to content

Alias Monolog\Formatter\JsonFormatter instead of creating a new one #11301

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

Merged
merged 1 commit into from
Jul 10, 2019

Conversation

B-Galati
Copy link
Contributor

@B-Galati B-Galati commented Apr 5, 2019

Let me know if I am wrong but I thought it would be more reliable to use MonologBundle service instead of creating a new one.

@javiereguiluz javiereguiluz requested a review from xabbuh April 5, 2019 14:31
@xabbuh xabbuh added this to the 3.4 milestone Apr 5, 2019
@xabbuh
Copy link
Member

xabbuh commented Apr 5, 2019

Should we remove the whole service instead and use monolog.formatter.json in the config below instead?

@B-Galati
Copy link
Contributor Author

B-Galati commented Apr 5, 2019

This solution leverage autowiring which can be "cool" to showcase in the doc.

@B-Galati B-Galati changed the base branch from 4.2 to 3.4 April 5, 2019 18:51
@B-Galati
Copy link
Contributor Author

B-Galati commented Apr 5, 2019

I updated base branch to 3.4

@OskarStark OskarStark added the ⭐️ EU-FOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming label Apr 7, 2019
@javiereguiluz
Copy link
Member

We need some Monolog expert to review this. Thanks!

@B-Galati
Copy link
Contributor Author

B-Galati commented Jul 4, 2019

Friendly ping @Seldaek @lyrixx @stof

@lyrixx
Copy link
Member

lyrixx commented Jul 4, 2019

This change is valid, but I have the same concern as @xabbuh

anyway 👍

javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this pull request Jul 10, 2019
…creating a new one (B-Galati)

This PR was merged into the 3.4 branch.

Discussion
----------

Alias Monolog\Formatter\JsonFormatter instead of creating a new one

Let me know if I am wrong but I thought it would be more reliable to use MonologBundle service instead of creating a new one.

Commits
-------

aecac40 Alias Monolog\Formatter\JsonFormatter instead of creating a new one
@javiereguiluz javiereguiluz merged commit aecac40 into symfony:3.4 Jul 10, 2019
@javiereguiluz
Copy link
Member

@B-Galati thanks for this contribution. However, while merging, we did some changes to finally remove the service and use the built-in Monolog service instead in the configuration. Both Christian and Grégoire expressed some concerns, so I think it's OK to do that. Thanks.

@B-Galati
Copy link
Contributor Author

@javiereguiluz Thanks for the heads up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⭐️ EU-FOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming Logger Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants