----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122428/#review75474 -----------------------------------------------------------
Ship it! Looks good src/personsmodel.cpp <https://git.reviewboard.kde.org/r/122428/#comment52193> I was always told that making private d-pointer a QObject is a very bad idea as it makes things unnecessarily heavy. Do we really need this as a QObject? If so, then it misses the Q_OBJECT macro too src/personsmodel.cpp <https://git.reviewboard.kde.org/r/122428/#comment52191> While moving this, it should also get renamed, it was meant "data for KABC::Addresee", this name makes no sense now; I think dataForPerson would suffice (or just put it back in data()? imo this separation does not serve much purpose now..) src/personsmodel.cpp <https://git.reviewboard.kde.org/r/122428/#comment52192> This looks like it lost 3 indentation spaces - Martin Klapetek On Feb. 4, 2015, 5:55 p.m., Aleix Pol Gonzalez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122428/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2015, 5:55 p.m.) > > > Review request for KDEPIM and Telepathy. > > > Repository: libkpeople > > > Description > ------- > > Move the private methods into the p-impl. First, it's the correct thing to > do. Secondly, it was pulling an awkward dependency on abstractcontact.h from > the backends into a public header. > > > Diffs > ----- > > src/CMakeLists.txt d8b4875 > src/backends/abstractcontact.h 4d1edcd > src/personsmodel.h 239f4ed > src/personsmodel.cpp bc2bee6 > > Diff: https://git.reviewboard.kde.org/r/122428/diff/ > > > Testing > ------- > > Builds, tests pass, ktp-contactlist still works. > > > Thanks, > > Aleix Pol Gonzalez > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
