D10578: balooctl monitor: Wait for dbus interface

Michael Heidelbach noreply at phabricator.kde.org
Sat Feb 17 10:12:16 UTC 2018


michaelh added inline comments.

INLINE COMMENTS

> alexeymin wrote in monitorcommand.cpp:45
> Strange formatting of commas in above 3 lines. It looks OK in constructor, where member initialization may be added later, but here, when the parameter count is fixed, no need to start new line with a comma. I'd suggest to keep as it was before

You're right. I just looked at the example in Qt's coding style rules. In fact it is operators BOL, commas EOL.
I'm ok with putting the commas at EOL (it's the rule, after all). Since to me it is more readable this way I'd like to wait for a second opinion or your objection.

> alexeymin wrote in monitorcommand.cpp:54
> `delete m_dbusInterface;` after this line, because below you are creating a new object? Each every 50 ms? Memory leaks here

Yeah, massively. Thanks for pointing that out.

> alexeymin wrote in monitorcommand.h:28
> Why dod you move local includes up? Usually they are below globals

Not in baloo's code it seems. So it's for consistency.

REPOSITORY
  R293 Baloo

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

To: michaelh, dfaure
Cc: alexeymin, #frameworks, ashaposhnikov, michaelh, spoorun, nicolasfella
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180217/99c755de/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list