Skip to content

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