> 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.) > > Vishesh Handa wrote: > 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. > > Harald Sitter wrote: > Talk to dfaure if he considers it ok to remove all interfaces and then > remove all interfaces and replace the one pseudo-used in plasma with an empty > file. Since they are not public API they ought not be installed so we'll > eliminate this source of SIC headache for the future, and since plasma > doesn't actively use the file it references, a stub place holder ought to > prevent that from exploding. > > David Faure wrote: > Yes, please do whatever is needed for the last plasma release to keep > building with KF 5.14, even if that means installing an empty file. > > For the hypothetical outside users, we'll just have to say "you were not > supposed to use this", just like someone using a Qt private header isn't > supposed to. > It was maybe less clear for the case of the baloo dbus interface, but > anyway, it's clear now, and we indeed don't want to remain stuck due to that > hypothetical case.
Solved by https://git.reviewboard.kde.org/r/125124/ - David ----------------------------------------------------------- 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 <<
