Review Request 114559: Only try loading data from the relevant data source

[David Edmundson]

> On Dec. 20, 2013, 10:49 a.m., Alexandr Akulich wrote:
> > src/basepersonsdatasource.h, line 49
> > <http://git.reviewboard.kde.org/r/114559/diff/1/?file=226653#file226653line49>
> >
> >     So, one plugin can provide only one source? I'm ok for that, but as it ABI changes, we should think again - what's if plugin support several simular sources via one codebase? Though, such code can be extracted to library.

I can't see it being a problem - it's a name to match a hidden identifier to a plugin so we know which plugin to query to get data.
Inside a plugin we can do multiple things, like Akonadi abstracts all sorts of crazy real sources - but to kpeople it's all just "akonadi"

Maybe source is a bad name?


> On Dec. 20, 2013, 10:49 a.m., Alexandr Akulich wrote:
> > src/personpluginmanager.h, line 38
> > <http://git.reviewboard.kde.org/r/114559/diff/1/?file=226655#file226655line38>
> >
> >     The pointer sign should be moved to the variable

I think this is right isn't it? There is no variable name on a return type. 


- David


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


On Dec. 20, 2013, 12:27 a.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114559/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2013, 12:27 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: libkpeople
> 
> 
> Description
> -------
> 
> Only try loading data from the relevant data source
> 
> i.e only query akonadi for contacts starting with akonadi://
> 
> 
> Thoughts: 
>  - should this be a pure virtual method in the data source or a property in the .desktop file?
> 
>  - the parsing of the IDs to get the sourceId from the url-looking string is a bit rubbish, in a future patch I'll tidy this up with an ID class that doesn't have this horrible string parsing.
> 
> 
> Note this is an ABI break. 
> 
> 
> Diffs
> -----
> 
>   src/basepersonsdatasource.h 12f698e 
>   src/persondata.cpp 277fa6b 
>   src/personpluginmanager.h adc7b9d 
>   src/personpluginmanager.cpp e433bb5 
>   src/plugins/akonadi/akonadidatasource.h ef5c602 
>   src/plugins/akonadi/akonadidatasource.cpp b640d00 
> 
> Diff: http://git.reviewboard.kde.org/r/114559/diff/
> 
> 
> Testing
> -------
> 
> Opened PersonViewer
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20131220/1365b10c/attachment.html>