D9881: Replace deprecated QSignalMapper by lambda expressions

Christoph Roick noreply at phabricator.kde.org
Fri Jan 26 00:41:48 UTC 2018


croick marked an inline comment as done.
croick added inline comments.

INLINE COMMENTS

> mwolff wrote in midebuggerplugin.cpp:151
> the old code was odd here already, but shouldn't you first check whether the service is already known and a proxy exists?
> or at least ensure that its freed by calling `slotDBusServiceUnregistered`
> 
> also, add a comment here please saying that we use the proxy to map N services to the one slot that takes the proxy.
> 
> Finally, can't we just use old-style connect + `sender()` here to simplify the code? Yes, `sender()` isn't the nicest thing on earth, but it certainly is less code than this here.

Actually, since I didn't know what the thing is supposed to do, I checked the DrKonqi side of the code: The whole thing is never called, since DrKonqi never starts a service called "org.kde.drkonqi". There is a service foreseen called "org.kde.Krash", but it is not registered to the DBus.
So I think, I will drop those changes completely for now and fix this independently within DrKonqi and KDevelop in separate patches if you agree.

> mwolff wrote in registersview.cpp:244
> why not capture a and then call `a->text()` in the lambda?

Sure, if there is no `a`, there is no connection. Thanks.

REPOSITORY
  R32 KDevelop

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

To: croick, #kdevelop, mwolff
Cc: mwolff, apol, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180126/fc7e3431/attachment.html>


More information about the KDevelop-devel mailing list