> On Aug. 31, 2015, 8:45 a.m., Hrvoje Senjan wrote: > > src/dbus/CMakeLists.txt, line 26 > > <https://git.reviewboard.kde.org/r/124919/diff/2/?file=398321#file398321line26> > > > > This looks like a SiC change to me, no? e.g. plasma-desktop won't build > > with this commit > > Harald Sitter wrote: > Yeah unfortunately removing methods from the dbus interfaces and renaming > the interfaces files (file.index -> fileindexer) is as SiC as it gets. In > particular considering the interfaces are getting installed they are forming > part of the public API. > > Pinak Ahuja wrote: > * I didn't realize messing around with the D-Bus interfaces would be a > big deal. Forgive me for my ignorance, but what is a SiC change? From the > discussion above, SiC change seems to be a change in the public API? What is > the proper way to handle it. > > * Also I looked at plasma-desktop, it does generate a D-Bus interface for > baloo in baloo kcm but doesn't seem to be using it? Should I just remove it > from there? > > Vishesh Handa wrote: > They are not part of the public API. I refuse to place additional > restrictions which do not allow Baloo to evolve. > > I grepped the plasma code, and it seems the KCM does depend on the > interface in the dbus in the CMakeLists but does not actually use it > anywhere. I've fixed it plasma-desktop 5.4 branch. > > For this release, if you guys want we can still ship the old dbus > interface, but it won't work. > > Hrvoje Senjan wrote: > >They are not part of the public API. I refuse to place additional > restrictions which do not allow Baloo to evolve. > > Please see other similar reviews for another framework: > https://git.reviewboard.kde.org/r/122926/ > https://git.reviewboard.kde.org/r/122970/ > > If you install an interface, it's public, doesn't matter do you consider > it such or no ;-) > (It's not a matter 'do we want' it, but that's incorrect. There are > possible users outside projects.kde.org.)
It matters to me. Baloo guarentees ABI stability for the libraries and API stability for the C++ interfaces. That is all. If you want I can write this down in the README. The dbus interface is going to break, it makes more sense to break it by removing the file instead of silently failing. However, considering that plasma-desktop does have a dependency on it, I can ship an empty file to satisfy the requirement. Perhaps for the next 6 months or so. - Vishesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124919/#review84622 ----------------------------------------------------------- On Aug. 30, 2015, 4:32 p.m., Pinak Ahuja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124919/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2015, 4:32 p.m.) > > > Review request for Baloo and Vishesh Handa. > > > Repository: baloo > > > Description > ------- > > * Previously mainhub class was being used to forward D-Bus calls to relevant > objects > * Now each object that needs D-Bus communication is registered as a separate > D-Bus object and communication takes place directly. > * FileContentIndexer has been made a long lived class now to register a D-Bus > object for it. > > > Diffs > ----- > > src/dbus/CMakeLists.txt 7c37d94 > src/file/filecontentindexer.h eeaa93f > src/file/filecontentindexer.cpp 26f98a3 > src/file/fileindexscheduler.h 90c23c9 > src/file/fileindexscheduler.cpp 89587bc > src/file/indexerstate.h 2ed8ec9 > src/file/mainhub.h 08993c6 > src/file/mainhub.cpp dcfac30 > src/tools/baloo-monitor/CMakeLists.txt 1abb16a > src/tools/baloo-monitor/monitor.h 597690a > src/tools/baloo-monitor/monitor.cpp 485db87 > src/tools/balooctl/CMakeLists.txt 2823e65 > src/tools/balooctl/main.cpp 81bedf6 > src/tools/balooctl/monitor.h 2a39ba9 > src/tools/balooctl/monitor.cpp c5d3b50 > > Diff: https://git.reviewboard.kde.org/r/124919/diff/ > > > Testing > ------- > > baloo-monitor and balooctl seem to be working as before. > > > Thanks, > > Pinak Ahuja > >
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
