> On June 24, 2015, 10:54 p.m., Stefan Brüns wrote: > > 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.
Using property sounds like a good idea, it would certainly help in case the extractor is stuck and we open the monitor to check which file it was working with. Though I'm not sure if we need the complexity of keeping track of running monitors as I can't think of any use case in which we have more than one monitor running, though if such a need arises I can add that later on. - Pinak ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124162/#review81736 ----------------------------------------------------------- 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 <<
