----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114605/#review46072 -----------------------------------------------------------
Ship it! Good start. Couple remarks below...also not sure about the personpluginmanager.h method... src/autotests/fakecontactsource.h <https://git.reviewboard.kde.org/r/114605/#comment32871> Remove (all files) src/autotests/fakecontactsource.h <https://git.reviewboard.kde.org/r/114605/#comment32872> pointer signs src/autotests/fakecontactsource.cpp <https://git.reviewboard.kde.org/r/114605/#comment32873> Why this? src/autotests/fakecontactsource.cpp <https://git.reviewboard.kde.org/r/114605/#comment32874> Space src/autotests/fakecontactsource.cpp <https://git.reviewboard.kde.org/r/114605/#comment32875> Spaaaaace src/autotests/persondatatests.cpp <https://git.reviewboard.kde.org/r/114605/#comment32876> Could maybe possibly likely use better name - like changeContact1Email()? So it's clearer what it does. src/personmanager.cpp <https://git.reviewboard.kde.org/r/114605/#comment32868> Can you change the "QObject* parent" as well since you've already edited that line please src/personmanager.cpp <https://git.reviewboard.kde.org/r/114605/#comment32869> Should we hard-switch to QSQLITE? src/personpluginmanager.h <https://git.reviewboard.kde.org/r/114605/#comment32870> Yes it is. - Martin Klapetek On Dec. 22, 2013, 2:17 a.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114605/ > ----------------------------------------------------------- > > (Updated Dec. 22, 2013, 2:17 a.m.) > > > Review request for Telepathy. > > > Repository: libkpeople > > > Description > ------- > > Add tests for PersonData > > > Add method to override DataSources used for testing > > > Remove old nepomuk tests > > > Add hook to alter the personsDB used > > For making unit tests > > > Remove unused include > > > --------------- > > I haven't tested all the things I want; I really want to test the model > especially with lots of merging/unmerging > > > Diffs > ----- > > src/CMakeLists.txt 43e1008 > src/autotests/fakecontactsource.h PRE-CREATION > src/autotests/fakecontactsource.cpp PRE-CREATION > src/autotests/lib/CMakeLists.txt 2358e8b > src/autotests/lib/NepomukTestLibMacros.cmake.in 5189462 > src/autotests/lib/datagenerator.h 6e75b67 > src/autotests/lib/datagenerator.cpp 630585a > src/autotests/lib/nepomukserverrc.in 27b5c90 > src/autotests/lib/nepomuktest_export.h 38a8eaa > src/autotests/lib/testbase.h 6d2a6b3 > src/autotests/lib/testbase.cpp 954fc83 > src/autotests/lib/tools/CMakeLists.txt 2e24962 > src/autotests/lib/tools/dbus-session-begin.sh ac0e63a > src/autotests/lib/tools/dbus-session-end.sh 4a13a05 > src/autotests/lib/tools/nepomuk-sandbox-begin.sh.in a755590 > src/autotests/lib/tools/nepomuk-sandbox-end.sh.in e331cb0 > src/autotests/lib/tools/runNepomukTest.sh.in 18c73f9 > src/autotests/persondatatests.h PRE-CREATION > src/autotests/persondatatests.cpp PRE-CREATION > src/autotests/tests/CMakeLists.txt 63e6739 > src/autotests/tests/duplicatestests.h 2faed15 > src/autotests/tests/duplicatestests.cpp 4083825 > src/autotests/tests/persondatatests.h 90ec2a2 > src/autotests/tests/persondatatests.cpp bf0c470 > src/personmanager.cpp fd03ba7 > src/personmanager_p.h e334f53 > src/personpluginmanager.h adc7b9d > src/personpluginmanager.cpp e433bb5 > > Diff: https://git.reviewboard.kde.org/r/114605/diff/ > > > Testing > ------- > > Tested my new tests! > > > Thanks, > > David Edmundson > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
