> On July 31, 2011, 8:31 a.m., Volker Krause wrote:
> > agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp, line 137
> > <http://git.reviewboard.kde.org/r/102084/diff/3/?file=30169#file30169line137>
> >
> >     We should use KStandardDirs here for determining the storage locations, 
> > so it follows standard environment variables etc. See e.g. 
> > KStandardDirs::localxdgdatadir().

Done.


> On July 31, 2011, 8:31 a.m., Volker Krause wrote:
> > agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp, line 144
> > <http://git.reviewboard.kde.org/r/102084/diff/3/?file=30169#file30169line144>
> >
> >     some of the variables here could be made const

Probably yes, I'll add it.


> On July 31, 2011, 8:31 a.m., Volker Krause wrote:
> > agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp, line 167
> > <http://git.reviewboard.kde.org/r/102084/diff/3/?file=30169#file30169line167>
> >
> >     You changed given and family name to use "addNameX" methods, but not 
> > the other name parts (middle name, etc). Is this correct?

I think that depends on the cardinality in the ontologies, because some 
properties have setFoo only for lists and some only for strings. And the 
initial port used different headers, therefore the mixup. I'll fix it so it 
uses setFoo methods everywhere, creating lists from strings for list-only 
setters.


> On July 31, 2011, 8:31 a.m., Volker Krause wrote:
> > agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp, line 381
> > <http://git.reviewboard.kde.org/r/102084/diff/3/?file=30169#file30169line381>
> >
> >     Again a mix of addFoo/setFoo methods, is that intentional?

Explained above.


> On July 31, 2011, 8:31 a.m., Volker Krause wrote:
> > agents/nepomuk_contact_feeder/nepomukcontactfeeder.cpp, line 235
> > <http://git.reviewboard.kde.org/r/102084/diff/3/?file=30169#file30169line235>
> >
> >     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.

I'll see what I can do. For now I'll submit updated patch without this change 
and look into this later.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102084/#review5253
-----------------------------------------------------------


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

Reply via email to