----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102084/#review5253 -----------------------------------------------------------
Just a few minor comments inline, looks good afaict otherwise, I don't really know the details of the Nepomuk side of this though. agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp <http://git.reviewboard.kde.org/r/102084/#comment4761> We should use KStandardDirs here for determining the storage locations, so it follows standard environment variables etc. See e.g. KStandardDirs::localxdgdatadir(). agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp <http://git.reviewboard.kde.org/r/102084/#comment4760> some of the variables here could be made const agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp <http://git.reviewboard.kde.org/r/102084/#comment4758> You changed given and family name to use "addNameX" methods, but not the other name parts (middle name, etc). Is this correct? agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp <http://git.reviewboard.kde.org/r/102084/#comment4762> Hm, this is quite some copy&paste code here (not your fault, it was before already). Do you think we can refactor that to have less duplication? Not a blocker anyway, we can always do that later. agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp <http://git.reviewboard.kde.org/r/102084/#comment4763> Again a mix of addFoo/setFoo methods, is that intentional? - Volker On July 28, 2011, 11:57 a.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102084/ > ----------------------------------------------------------- > > (Updated July 28, 2011, 11:57 a.m.) > > > Review request for KDEPIM, Nepomuk and Christian Mollekopf. > > > Summary > ------- > > This is the port of Akonadi's Nepomuk-Contact-Feeder to the new Nepomuk > SimpleResource API. It follows Christian Mollekopf's NepomukFeederAgent port. > > > Diffs > ----- > > agents/CMakeLists.txt f742e86 > agents/nepomuk_contact_feeder/nepomukcontactfeeder.h 1e86efb > agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp bf32ed2 > agents/nepomukfeeder/nepomukfeederagentbase.cpp 7133c75 > > Diff: http://git.reviewboard.kde.org/r/102084/diff > > > Testing > ------- > > Created/updated a contact in KAddressBook, looked it up in Nepomuk database, > all data were present. > > > Thanks, > > Martin > >
_______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
