----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124162/#review81736 -----------------------------------------------------------
Some comments about the DBus interface you are exporting: I think you should make the current file name a property. Signal changes through the standard PropertiesChanged DBus signal. This way you can also add more properties later, but only use one common signal for property changes. It also allows a monitor client on startup to request the current file, without waiting for the next file. Instead of using an explicit flag for enabling/disabling the signaling, you should keep a list of registered monitors (you can get the name of the registering monitor with QDBusMessage::service()). Add a QDbusServiceWatcher to get notified if one of the monitors dies. - Stefan Brüns On June 24, 2015, 11:52 a.m., Pinak Ahuja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124162/ > ----------------------------------------------------------- > > (Updated June 24, 2015, 11:52 a.m.) > > > Review request for Baloo and Vishesh Handa. > > > Repository: baloo > > > Description > ------- > > Extractor now has a proper D-Bus interface with methods to enable/disable url > export. The monitor calles enable export on startup or when extractor starts > and disables export when it is closed. > > > Diffs > ----- > > src/file/extractor/app.h f8b8834 > src/file/extractor/app.cpp e288285 > src/file/extractor/main.cpp 58ddbeb > src/file/extractor/org.kde.baloo.extractor.xml PRE-CREATION > src/tools/baloo-monitor/CMakeLists.txt b80d129 > src/tools/baloo-monitor/main.cpp 9f108ea > src/tools/baloo-monitor/monitor.h b72a8da > src/tools/baloo-monitor/monitor.cpp 9957424 > > Diff: https://git.reviewboard.kde.org/r/124162/diff/ > > > Testing > ------- > > Tested using dbus-monitor, extractor only exports urls when monitor is > running. > > > Thanks, > > Pinak Ahuja > >
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
