Review Request 124176: Introduce KPeople contact querying service

[Aleix Pol Gonzalez]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124176/#review81784
-----------------------------------------------------------



src/service/service.h (line 33)
<https://git.reviewboard.kde.org/r/124176/#comment56100>

    Maybe org.kde.KPeople.LookupService?



src/service/service.h (line 41)
<https://git.reviewboard.kde.org/r/124176/#comment56094>

    I wouldn't call it hint, I'd call it property or key.
    
    It's not a hint in fact, we're not looking for all matches, but specifically a field. As it's phrased, it looks like it's an optimization.



src/service/service.cpp (line 46)
<https://git.reviewboard.kde.org/r/124176/#comment56099>

    It's more easily readable as:
    d->model->index(i).data(PersonsModel::PersonVCardRole)



src/service/service.cpp (line 48)
<https://git.reviewboard.kde.org/r/124176/#comment56095>

    Yes, make it mandatory.



src/service/service.cpp (line 49)
<https://git.reviewboard.kde.org/r/124176/#comment56096>

    I would just do:
    QVariantList dataList = person->customProperty("all-"+hint).toList();



src/service/service.cpp (line 57)
<https://git.reviewboard.kde.org/r/124176/#comment56097>

    contact.simplified() can be done only once outside of all loops.



src/service/service.cpp (line 58)
<https://git.reviewboard.kde.org/r/124176/#comment56098>

    It's more easily readable as:
    d->model->index(i).data(PersonsModel::PersonUriRole)


- Aleix Pol Gonzalez


On June 25, 2015, 3:33 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124176/
> -----------------------------------------------------------
> 
> (Updated June 25, 2015, 3:33 p.m.)
> 
> 
> Review request for KDEPIM and Telepathy.
> 
> 
> Repository: kpeople
> 
> 
> Description
> -------
> 
> This is a dbus service that runs in the background and returns contact ids for queried contact details, for example asking for a phone number would return a contact id from which a PersonData class can be constructed, thus making all contact data available for display.
> 
> This is very useful when eg. receiving a phone call, a tp approver starts up showing the incoming call (this needs to be instant) and once it's shown up, it should show a contact name for the number as soon as possible. However loading all the datasources and populating the model can be slow and speed is critical in this case. Therefore it's a dbus service which responds very fast and there's no need to initialize anything as it can be loaded in the background after system starts and it keeps itself always up to date.
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 233e7d9 
>   autotests/servicetest.h PRE-CREATION 
>   autotests/servicetest.cpp PRE-CREATION 
>   src/CMakeLists.txt 59bc915 
>   src/backends/abstractcontact.h ce22cbc 
>   src/backends/abstractcontact.cpp f01236b 
>   src/global.h e1d07ce 
>   src/global.cpp b3595ca 
>   src/service/CMakeLists.txt PRE-CREATION 
>   src/service/main.cpp PRE-CREATION 
>   src/service/org.kde.KPeople.service.in PRE-CREATION 
>   src/service/service.h PRE-CREATION 
>   src/service/service.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124176/diff/
> 
> 
> Testing
> -------
> 
> Unit test included; also tested with qdbus.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20150626/5ad7f7e8/attachment.html>