D10578: balooctl monitor: Wait for dbus interface

michaelh added inline comments. INLINE COMMENTS > dfaure wrote in monitorcommand.cpp:53 > Hmm, why not just use QDbusServiceWatcher's signals, to connect to > serviceRegistered, if the service isn't available when we start? The idea to do it that way came to late. :-) I'll make an update

D10578: balooctl monitor: Wait for dbus interface

dfaure added inline comments. INLINE COMMENTS > monitorcommand.cpp:53 > +while (!m_dbusInterface->isValid()) { > +QThread::msleep(50); > +m_dbusInterface->disconnect(); Hmm, why not just use QDbusServiceWatcher's signals, to connect to serviceRegistered, if the service

D10578: balooctl monitor: Wait for dbus interface

This revision was automatically updated to reflect the committed changes. Closed by commit R293:94da1850ec6c: balooctl monitor: Wait for dbus interface (authored by michaelh). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10578?vs=27416=27417 REVISION DETAIL

D10578: balooctl monitor: Wait for dbus interface

michaelh marked 11 inline comments as done. REPOSITORY R293 Baloo BRANCH wait-for-dbus (branched from master) REVISION DETAIL https://phabricator.kde.org/D10578 To: michaelh, dfaure, alexeymin Cc: cfeck, alexeymin, #frameworks, ashaposhnikov, michaelh, spoorun, nicolasfella

D10578: balooctl monitor: Wait for dbus interface

michaelh updated this revision to Diff 27416. michaelh added a comment. - balooctl monitor: Adhere to KDE coding-style REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10578?vs=27394=27416 BRANCH wait-for-dbus (branched from master) REVISION DETAIL

D10578: balooctl monitor: Wait for dbus interface

cfeck added inline comments. INLINE COMMENTS > michaelh wrote in monitorcommand.cpp:45 > 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

D10578: balooctl monitor: Wait for dbus interface

michaelh added a comment. In D10578#208132 , @alexeymin wrote: > There is also https://techbase.kde.org/Policies/Frameworks_Coding_Style. ... and it refers to Qt. > I've never seen commas at the start of the line. It's part of

D10578: balooctl monitor: Wait for dbus interface

alexeymin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH wait-for-dbus (branched from master) REVISION DETAIL https://phabricator.kde.org/D10578 To: michaelh, dfaure, alexeymin Cc: alexeymin, #frameworks, ashaposhnikov, michaelh,

D10578: balooctl monitor: Wait for dbus interface

alexeymin added a comment. There is also https://techbase.kde.org/Policies/Frameworks_Coding_Style. Guess I was wrong about `#include`s order, but honestly I've never seen commas at the start of the line. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D10578 To:

D10578: balooctl monitor: Wait for dbus interface

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

D10578: balooctl monitor: Wait for dbus interface

michaelh updated this revision to Diff 27394. michaelh marked an inline comment as done. michaelh added a comment. - balooctl monitor: Fix memory leak - balooctl monitor: Whitespace cleanup REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE

D10578: balooctl monitor: Wait for dbus interface

alexeymin added a comment. Looks almost fine, just fix these: INLINE COMMENTS > monitorcommand.cpp:45 > +, QDBusConnection::sessionBus() > +, this > +); Strange formatting of commas in above 3 lines. It looks OK in constructor, where member initialization may be added

D10578: balooctl monitor: Wait for dbus interface

michaelh added a comment. In D10578#207745 , @alexeymin wrote: > Can you add simple instructions how to reproduce full testing scenario? See test plan REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D10578 To:

D10578: balooctl monitor: Wait for dbus interface

michaelh updated this revision to Diff 27360. michaelh marked an inline comment as done. michaelh edited the summary of this revision. michaelh edited the test plan for this revision. michaelh added a comment. - balooctl monitor: Use QCoreApplication::exit() - balooctl monitor: Explicit

D10578: balooctl monitor: Wait for dbus interface

michaelh edited the test plan for this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D10578 To: michaelh, dfaure Cc: alexeymin, #frameworks, ashaposhnikov, michaelh, spoorun, nicolasfella

D10578: balooctl monitor: Wait for dbus interface

alexeymin added a comment. Can you add simple instructions how to reproduce full testing scenario? INLINE COMMENTS > monitorcommand.cpp:75 > +//TODO: Wait again for dbus interface > +QCoreApplication::instance()->exit(0); > +}); No need to call `instance()`

D10578: balooctl monitor: Wait for dbus interface

michaelh created this revision. michaelh added a reviewer: dfaure. michaelh added a project: Baloo. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. michaelh requested review of this revision. REVISION SUMMARY If baloo is not running,