Review Request 114559: Only try loading data from the relevant data source
Martin Klapetek
martin.klapetek at gmail.com
Fri Dec 20 15:48:00 UTC 2013
> On Dec. 20, 2013, 11: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
>
> 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.
We've always used the format David is using. Let's stick with that.
- Martin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/114559/#review45952
-----------------------------------------------------------
On Dec. 20, 2013, 1: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, 1: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/dd120f5b/attachment-0001.html>
More information about the KDE-Telepathy
mailing list