D10712: balooctl monitor: Resume to wait for service
michaelh created this revision. michaelh added reviewers: Baloo, Frameworks, dfaure, alexeymin. Restricted Application added a project: Frameworks. michaelh requested review of this revision. REVISION SUMMARY Keep monitoring until terminated by ctrl+c TEST PLAN While monitor is running various permutations of - `balooctl stop` - `balooctl start` - `balooctl restart` - `echo otto >> ~/Documents/otto.csv` REPOSITORY R293 Baloo BRANCH servicewatcher_pingpong REVISION DETAIL https://phabricator.kde.org/D10712 AFFECTED FILES src/tools/balooctl/monitorcommand.cpp src/tools/balooctl/monitorcommand.h To: michaelh, #baloo, #frameworks, dfaure, alexeymin Cc: michaelh
D10712: balooctl monitor: Resume to wait for service
michaelh added a project: Baloo. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D10712 To: michaelh, #baloo, #frameworks, dfaure, alexeymin Cc: ashaposhnikov, michaelh, spoorun, nicolasfella, alexeymin
D10712: balooctl monitor: Resume to wait for service
michaelh added a comment. *ping* REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D10712 To: michaelh, #baloo, #frameworks, dfaure, alexeymin Cc: ashaposhnikov, michaelh, spoorun, nicolasfella, alexeymin
D10712: balooctl monitor: Resume to wait for service
mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.
some minor comments, otherwise lgtm
INLINE COMMENTS
> monitorcommand.cpp:45
> +connect(m_dbusServiceWatcher, &QDBusServiceWatcher::serviceUnregistered,
> [this]() {
> +m_err << i18nc("Application", "%1 died", "Baloo") << endl;
> +});
just make it i18n("Baloo died"), though I personally think that message should
be improved - maybe "Baloo became unavailable" ?
also, call `balooIsNotAvailable()` from the slot here, that way you safe one
connection. Or just move the m_err output to the `balooIsNotAvailable` slot.
> monitorcommand.cpp:61
> +if (m_dbusInterface->isValid()) {
> +m_err << i18n("Press ctrl+c to exit monitor") << endl;
> +balooIsAvailable();
`Press Ctrl + C to stop monitoring`
> monitorcommand.cpp:65
> +balooIsNotAvailable();
> +}
> +}
add a
m_dbusServiceWatcher->setWatchMode(QDBusServiceWatcher::WatchForOwnerChange);
> monitorcommand.cpp:71
> +m_dbusInterface->unregisterMonitor();
> +m_err << i18nc("Application", "Waiting for %1 to start", "Baloo") <<
> endl;
> +m_err << i18n("Press ctrl+c to exit monitor") << endl;
see above: `i18n("Waiting for Baloo to start")`
> monitorcommand.cpp:72
> +m_err << i18nc("Application", "Waiting for %1 to start", "Baloo") <<
> endl;
> +m_err << i18n("Press ctrl+c to exit monitor") << endl;
> +
see above
> monitorcommand.cpp:74
> +
> +
> m_dbusServiceWatcher->setWatchMode(QDBusServiceWatcher::WatchForRegistration);
> +}
remove this
> monitorcommand.cpp:80
> +m_dbusInterface->registerMonitor();
> +m_err << i18nc("Application", "%1 is running", "Baloo") << endl;
> +
see above: i18n("Baloo is running")
> monitorcommand.cpp:82
> +
> +
> m_dbusServiceWatcher->setWatchMode(QDBusServiceWatcher::WatchForUnregistration);
> +
remove this
REPOSITORY
R293 Baloo
REVISION DETAIL
https://phabricator.kde.org/D10712
To: michaelh, #baloo, #frameworks, dfaure, alexeymin, mwolff
Cc: mwolff, ashaposhnikov, michaelh, spoorun, nicolasfella, alexeymin
D10712: balooctl monitor: Resume to wait for service
michaelh updated this revision to Diff 28909. michaelh marked 8 inline comments as done. michaelh edited the test plan for this revision. michaelh added a comment. - Apply suggested changes REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10712?vs=27678&id=28909 BRANCH resume-wait (branched from master) REVISION DETAIL https://phabricator.kde.org/D10712 AFFECTED FILES src/tools/balooctl/monitorcommand.cpp src/tools/balooctl/monitorcommand.h To: michaelh, #baloo, #frameworks, dfaure, alexeymin, mwolff Cc: mwolff, ashaposhnikov, michaelh, spoorun, nicolasfella, alexeymin
D10712: balooctl monitor: Resume to wait for service
michaelh added inline comments.
INLINE COMMENTS
> mwolff wrote in monitorcommand.cpp:45
> just make it i18n("Baloo died"), though I personally think that message
> should be improved - maybe "Baloo became unavailable" ?
>
> also, call `balooIsNotAvailable()` from the slot here, that way you safe one
> connection. Or just move the m_err output to the `balooIsNotAvailable` slot.
Can be removed completely.
REPOSITORY
R293 Baloo
REVISION DETAIL
https://phabricator.kde.org/D10712
To: michaelh, #baloo, #frameworks, dfaure, alexeymin, mwolff
Cc: mwolff, ashaposhnikov, michaelh, spoorun, nicolasfella, alexeymin
D10712: balooctl monitor: Resume to wait for service
mwolff accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH resume-wait (branched from master) REVISION DETAIL https://phabricator.kde.org/D10712 To: michaelh, #baloo, #frameworks, dfaure, alexeymin, mwolff Cc: mwolff, ashaposhnikov, michaelh, spoorun, nicolasfella, alexeymin
D10712: balooctl monitor: Resume to wait for service
This revision was automatically updated to reflect the committed changes. Closed by commit R293:add3284efdca: balooctl monitor: Resume to wait for service (authored by michaelh). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10712?vs=28909&id=28933 REVISION DETAIL https://phabricator.kde.org/D10712 AFFECTED FILES src/tools/balooctl/monitorcommand.cpp src/tools/balooctl/monitorcommand.h To: michaelh, #baloo, #frameworks, dfaure, alexeymin, mwolff Cc: mwolff, ashaposhnikov, michaelh, spoorun, nicolasfella, alexeymin
