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



autotests/fakecontactsource.cpp
<https://git.reviewboard.kde.org/r/121639/#comment50651>

    currently this is unused, which means we can't be  testing where we have 
multiple properties 
    
    also I don't really understand what this line would be doing, turning a 
variant into a string and then into a stringlist and then into a variant?



autotests/fakecontactsource.cpp
<https://git.reviewboard.kde.org/r/121639/#comment50650>

    I would have expected this to use the static strings in AbstractContact



src/defaultcontactmonitor.cpp
<https://git.reviewboard.kde.org/r/121639/#comment50649>

    given we now control AbstractContact we can put the ID in that. 
    
    Will simplify a few APIs



src/metacontact.cpp
<https://git.reviewboard.kde.org/r/121639/#comment50655>

    merging behaviour isn't this simple.
    It change on the property, especially for cardinality.
    
    if I request a name I don't want to get a list, and it's especially 
important as we can use the redesign to not bother querying for data we're not 
going to show.
    
    It also makes the usage more complex where clients now have to start 
handling lists, and grouping all emails (now a list of lists) together.



src/metacontact.cpp
<https://git.reviewboard.kde.org/r/121639/#comment50652>

    deliberately public?



src/persondata.cpp
<https://git.reviewboard.kde.org/r/121639/#comment50656>

    We're converting a list of variants to a string?
    
    I'm not entirely sure what the behaviour will be, but I doubt it's what we 
want.


- David Edmundson


On Dec. 24, 2014, 1:07 a.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121639/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2014, 1:07 a.m.)
> 
> 
> Review request for KDEPIM, Telepathy, Christian Mollekopf, David Edmundson, 
> and Martin Klapetek.
> 
> 
> Repository: libkpeople
> 
> 
> Description
> -------
> 
> Last week we had a couple of discussions regarding KPeople relationship with 
> KDE PIM and KContacts and we agreed that we want to keep KContacts as a 
> framework that is good at dealing with vCards, which leaves KPeople without a 
> good way to send around its data.
> 
> This patch proposes an internal AbstractContact class that the backends will 
> have to re-implement. At the moment it only requires a "customProperty" that 
> lets us fetch whatever is needed.
> Furthermore, it also includes a set of convenience methods in PersonData to 
> have a type-safe API.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3df5f0b 
>   autotests/CMakeLists.txt 676f170 
>   autotests/fakecontactsource.h 981fe03 
>   autotests/fakecontactsource.cpp 269f94f 
>   autotests/persondatatests.cpp 47a6213 
>   examples/CMakeLists.txt 74bd188 
>   examples/duplicates.cpp 72ff53b 
>   src/CMakeLists.txt 534066b 
>   src/backends/abstractcontact.h PRE-CREATION 
>   src/backends/abstractcontact.cpp PRE-CREATION 
>   src/backends/abstractpersonaction.h 703063e 
>   src/backends/abstractpersonaction.cpp be93d90 
>   src/backends/allcontactsmonitor.h 47e5dfd 
>   src/backends/allcontactsmonitor.cpp e517f17 
>   src/backends/basepersonsdatasource.h 3c25e28 
>   src/backends/basepersonsdatasource.cpp b794917 
>   src/backends/contactmonitor.h cf25953 
>   src/backends/contactmonitor.cpp ceb5d74 
>   src/declarative/personactionsmodel.cpp 4b34e15 
>   src/defaultcontactmonitor.cpp 36c31a7 
>   src/defaultcontactmonitor_p.h 8fc2812 
>   src/duplicatesfinder.cpp c6a1014 
>   src/global.h 9540623 
>   src/global.cpp 3ef834b 
>   src/match.cpp 3a93aba 
>   src/match_p.h 5727270 
>   src/metacontact.cpp bba6b65 
>   src/metacontact_p.h 0d738cc 
>   src/persondata.h 66b19a3 
>   src/persondata.cpp dab9f61 
>   src/personpluginmanager.cpp f249b5f 
>   src/personsmodel.h db907d4 
>   src/personsmodel.cpp ca6e972 
>   src/plugins/akonadi/akonadidatasource.cpp ae4ace1 
>   src/widgets/abstractfieldwidgetfactory.h 1d2d8aa 
>   src/widgets/persondetailsdialog.cpp 8ca0a99 
>   src/widgets/persondetailsview.cpp b7535bd 
>   src/widgets/plugins/emaildetailswidget.h 72a96ee 
>   src/widgets/plugins/emaildetailswidget.cpp 839c5f6 
>   src/widgets/plugins/emails.h ede1686 
>   src/widgets/plugins/emails.cpp d69101b 
> 
> Diff: https://git.reviewboard.kde.org/r/121639/diff/
> 
> 
> Testing
> -------
> 
> Builds, the test passes. ktp tests work, somewhat.
> Also there's few tests altogether.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to