[Kde-pim] Review Request 121639: Split out KF5Contacts dependency

[David Edmundson]

-----------------------------------------------------------
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 PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/