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