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

[Alexandr Akulich]

> On Dec. 20, 2013, 4:49 p.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
> 
> David Edmundson wrote:
>     I think this is right isn't it? There is no variable name on a return type.
> 
> Alexandr Akulich wrote:
>     I reads http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Whitespace as generic rule, that order us to write it as
>     static BasePersonsDataSource *dataSource(const QString &sourceId);
>     IMO method signatures shouldn't go against this rule.
>     I'll be glad if someone clarify it for me.
> 
> Martin Klapetek wrote:
>     We've always used the format David is using. Let's stick with that.

I have seen such style in ktp.
It's sadly going against Qt. E.g. look at QCoreApplication methods:
static QCoreApplication *instance();

I'm against KTp style, but okey, I'll accept and use it. Is it reflected somewhere in wiki?


- Alexandr


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


On Dec. 20, 2013, 6: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, 6: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/ef05a8a2/attachment.html>