[Kde-pim] Review Request 121096: convert qDebug and qWarning to kDebug and kWarning

[René J.V. Bertin]

> On Nov. 11, 2014, 7:29 p.m., Laurent Montel wrote:
> > agents/newmailnotifier/newmailnotifiershowmessagejob.cpp, line 48
> > <https://git.reviewboard.kde.org/r/121096/diff/2/?file=327750#file327750line48>
> >
> >     ?????? kFatal ?????
> 
> René J.V. Bertin wrote:
>     Yes, that seems appropriate here. It's not that it does anything more than just printing the message ...
> 
> Laurent Montel wrote:
>     No it's not !
>     It's my code and I don't want a kFatal(). So don't presume things that you didn't write.
> 
> Martin Klapetek wrote:
>     kFatal means "die on the spot", ie. crash. That's why it's "fatal" :)

I beg to differ. From kdebug.h, KDE git/4.14.3:

````C++
/**
 * \relates KGlobal
 * Returns a fatal error stream. You can use it to print fatal error
 * information.
 * @param area an id to identify the output, KDE_DEFAULT_DEBUG_AREA for default
 */
static inline QDebug kFatal(int area = KDE_DEFAULT_DEBUG_AREA)
{ return kDebugStream(QtFatalMsg, area); }
static inline QDebug kFatal(bool cond, int area = KDE_DEFAULT_DEBUG_AREA)
{ return cond ? kFatal(area) : kDebugDevNull(); }

```

In other words, kFatal doesn't abort any more than qFatal does. As to the idea that fatal would be only conditions that lead to a crash, I don't agree with that. Fatal is a situation in which you have to give up whatever you were trying to do, what happens afterwards is irrelevant. The software could sing a lullaby and then shoot the user for asking the impossible :) Or it could just continue running with the parts that do work, as seems to be the case here.

But if kmail not starting when requested is not fatal enough an event, would a kWarning be acceptable?


- René J.V.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121096/#review70245
-----------------------------------------------------------


On Nov. 11, 2014, 4:53 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121096/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2014, 4:53 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Repository: kdepim-runtime
> 
> 
> Description
> -------
> 
> This patch does as said in the summary. Rationale is to avoid log pollution, storing potentially sensitive data in unexpected locations and waste of CPU cycles.
> First impressions are that disabling debug output (which is now under kdebugdialog control) does indeed make KMail considerably "snappier" (at least when combined with akonadi built with -DQT_NO_DEBUG_OUTPUT).
> 
> This also makes kdepim-runtime adhere more closely to KDE debugging output guidelines.
> 
> 
> Diffs
> -----
> 
>   qml/kde/tests/qml_moves/mainwindow.cpp 1324575 
>   qml/kde/tests/qmlbreadcrumbnavigation/checkableitemproxymodel.cpp f61485a 
>   qml/kde/tests/qmlbreadcrumbnavigation/kmodelindexproxymapper.cpp c464938 
>   qml/kde/tests/qmlbreadcrumbnavigation/kproxyitemselectionmodel.cpp 6d176fb 
>   qml/kde/tests/qmlbreadcrumbnavigation/qmllistselectionmodel.cpp dbc794e 
>   resources/imap/tests/testsubscriptiondialog.cpp 9653b76 
>   resources/kolabproxy/kolabhandler.cpp 0574464 
>   resources/kolabproxy/upgradejob.cpp 74f80fe 
>   resources/maildir/libmaildir/maildir.cpp 9ad4a66 
>   resources/mixedmaildir/kmindexreader/tests/testidxreader.cpp d51cc9d 
>   agents/newmailnotifier/newmailnotifiershowmessagejob.cpp cbbcbed 
>   qml/kde/kdeintegrationplugin.cpp c1360b4 
>   qml/kde/tests/qml_moves/dynamictreemodel.cpp 5537bb7 
>   resources/mixedmaildir/tests/itemfetchtest.cpp 5372ce5 
>   resources/openxchange/openxchangeresource.cpp 377b908 
>   resources/openxchange/oxa/incidenceutils.cpp 1f0592b 
>   resources/pop3/tests/fakeserver/fakeserver.cpp 2201045 
>   resources/pop3/tests/pop3test.cpp f6b0f2a 
> 
> Diff: https://git.reviewboard.kde.org/r/121096/diff/
> 
> 
> Testing
> -------
> 
> Ubuntu 14.04 with kdelibs 4.14.2 .
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/