[Differential] [Commented On] D2295: improve logging for kscreen

sebas (Sebastian Kügler) noreply at phabricator.kde.org
Thu Jul 28 10:49:34 UTC 2016


sebas added inline comments.

INLINE COMMENTS

> bshah wrote in testlog.cpp:88
> nitpick - cAPs. ;-)

It's a test, this way it covers strange capitalization. :)

> davidedmundson wrote in log.cpp:46
> Question:
> What's the threading policy on libkscreen?
> 
> This isn't reentrant (which might be fine)

No threads in libkscreen. It'll probably blow up in many other cases already.

> davidedmundson wrote in log.cpp:71
> you probably want to manually force the logging category to enable itself if this env var is set.
> 
> or get rid of this env var, and just check if the logging category is enabled.

Good point. I'll check if it's needed (I was kind of assuming that the policy checks are handled in the default handler, so we're not even bothered.)

> davidedmundson wrote in log.cpp:137
> You're going to need a file lock round this aren't you?
> The entire point is that two processes will be writing in here at the same time.

It's actually complexity I want to avoid. If the logs are messed up, bad luck, but introducing wait locks and the likes brings in so much complexity, I think it's better to avoid here  until it becomes a problem.

REPOSITORY
  rLIBKSCREEN KScreen Library

BRANCH
  sebas/log

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

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

To: sebas, #plasma, bshah
Cc: davidedmundson, bshah, plasma-devel, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160728/ebb63679/attachment-0001.html>


More information about the Plasma-devel mailing list