Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Remove an unneeded logging article#8770
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ghost commentedNov 29, 2017
Build failure is unrelated to this PR. This has been fixed in#8772. |
xabbuh commentedNov 30, 2017
This looks good. Can you also add an entry to the redirection map? |
javiereguiluz left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I like the idea, but I'm not sure about the proposal. This is a very unimportant option. It doesn't deserve an entire section in my opinion. What if we just add a very short mention to it in this reference:https://symfony.com/doc/current/reference/configuration/monolog.html ?
weaverryan commentedNov 30, 2017
Yea, I agree. We just need to add |
ghost commentedDec 1, 2017
I am unsure about the comments. They are long right now, and that is the incorrect way according to the feedback@weaverryan gave in#8770 (comment). See commit4b3ed43 about the changes. I'd love to get better comment texts to include instead of these long lines cluttering the article since this is such a small change. |
| datetime field of the log messages from | ||
| microsecond to second. Avoiding a call to the | ||
| microtime(true) function and the subsequent parsing.--> | ||
| <monolog:configuse-microseconds="true"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No need to put the comments in XML also. We typically just put the comments in YAML - it's the most visible and avoids duplication :)
| # the precision in the datetime field of the log messages from | ||
| # microsecond to second. Avoiding a call to the microtime(true) | ||
| # function and the subsequent parsing. | ||
| use_microseconds:true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I agree! This is a little long :). How about:
Set to false to use seconds (instead of microseconds) in the logs (gives a small performance boost)javiereguiluz left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@ricknox thanks for this contribution and for your patience during the review process.
ghost commentedDec 4, 2017
@javiereguiluz No problem. It is good that we all pay attention so that the quality of the documentation remains high. |
weaverryan commentedDec 5, 2017
Great improvement! Thank you Ricardo! |
This is related to#8684. Sorry for the spelling error in the commit, didn't had my coffee yet ;)