-
-
Notifications
You must be signed in to change notification settings - Fork 149
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
Refactor logging #176
Refactor logging #176
Conversation
It should be called only once
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.
This is my favorite commit.
@@ -276,8 +248,6 @@ void Logger::appendLog(LogLevel level, const QString &message, bool newline) | |||
*/ | |||
void Logger::directLog(LogLevel level, const QString &message, bool newline) |
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.
Hi @jsbackus,
Could you help me with this one?
Do you remember what was the reason of differentiating between directLog
and appendLog
?
Were there any performance issues with logs causing these optimizations?
Hi @pktiuk, hmm.. no, that must have been before my time. I didn't have
much of a chance to do any tuning.
…On Sat, Jul 10, 2021 at 4:20 PM Paweł Kotiuk ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/logger.cpp
<#176 (comment)>:
> @@ -276,8 +248,6 @@ void Logger::appendLog(LogLevel level, const QString &message, bool newline)
*/
void Logger::directLog(LogLevel level, const QString &message, bool newline)
Hi @jsbackus <https://github.com/jsbackus>,
Could you tell me with this one?
Do you remember what was the reason of differentiating between directLog
and appendLog?
Were there any performance issues with logs causing these optimizations?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#176 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPMVM52JE54YTDVDJNYTW3TXCTQJANCNFSM473BDCOQ>
.
--
Jeff Backus
***@***.***
http://github.com/jsbackus
|
This task is too big to do it at once. It will be more manageable if it would be split to pieces. |
Fixes some logging issues:
qDebug
andLogger::LogDebug
)qInstallMessageHandler
in every method is a bad idea, it should be called only ONCEPart of #177