[Differential] [Changed Subscribers] D3323: Make it possible for plasmashell to log into kscreen.log

[graesslin (Martin Gräßlin)]

graesslin added inline comments.

INLINE COMMENTS

> shellcorona.cpp:87
>  static const int s_configSyncDelay = 10000; // 10 seconds
> +QtMessageHandler sDefaultMessageHandler = nullptr;
> +

static

> shellcorona.cpp:95
> +    QString logMessage = QString("\n%1 ; %2 : %4").arg(timestamp, _cat, msg);
> +    QString logFile = QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation) + "/kscreen/kscreen.log";
> +    QFile file(logFile);

Just saying: I don't like this writing to a dedicated file per session. We are starting to introduce a second xsession-errors.

Also I don't like that we have a duplication for the log message handling here between kscreen and plasma-workspace. If we want to start having additional applications write to the kscreen log, libkscreen should expose an API for it.

I also have to ask why debug messages like "plasmashell starting" should go into a "kscreen.log". How is that in any way related to kscreen? This is semantically not correct. I understand that you want everything multi screen related in one log file, but then please get the semantics correct.

> shellcorona.cpp:125
> +
> +    qCDebug(KSCREEN_PLASMASHELL) << "plasmashell starting";
>      setupWaylandIntegration();

this is not screen related

REPOSITORY
  rPLASMAWORKSPACE Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D3323

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: sebas, mart
Cc: graesslin, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161110/587e61ae/attachment.html>