Review Request 121639: Split out KF5Contacts dependency

[Martin Klapetek]

> On Jan. 5, 2015, 5:49 p.m., Martin Klapetek wrote:
> > src/persondata.h, lines 100-106
> > <https://git.reviewboard.kde.org/r/121639/diff/3/?file=337578#file337578line100>
> >
> >     Can we make this more explicit? I'd like to avoid clients always having to parse this and check for the content, so I'd suggest something like 
> >     
> >     "QPixmap picture() const" (and always load it if it's url)
> >     "QUrl pictureUrl() const"
> >     
> >     Most of the time QPixmap directly will be used anyway
> >     
> >     ...actually there's already photo(), so why would we want another QPixmap method?
> 
> Aleix Pol Gonzalez wrote:
>     Yes, but then people will have to do something like pictureUrl.isEmpty or picture.isNull.
>     
>     But yes, I agree it's something to look into.
> 
> Martin Klapetek wrote:
>     The original idea with photo() is to always provide a pixmap - if there's none set, return the dummy generic person shape. So using the pixmap method would then always be safe.
> 
> Aleix Pol Gonzalez wrote:
>     There's also a ::photo() method that does what your QPixmap::picture says, returning a QPixmap. Then there's ::picture with all the possible alternatives. I think it's handy to have them like this instead of having to juggle between the 2 methods.

But what's the point of the ::picture() method if there's already ::photo() that will /always/ get you a valid pixmap? When would I want to use picutre()?


- Martin


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


On Jan. 7, 2015, 1:21 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121639/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2015, 1:21 p.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
> -----
> 
>   src/widgets/plugins/emaildetailswidget.cpp 839c5f6 
>   src/widgets/plugins/emails.h ede1686 
>   src/widgets/plugins/emails.cpp d69101b 
>   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/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 0a3b00d 
>   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 40156a9 
>   src/personpluginmanager.cpp 59f5a48 
>   src/personsmodel.h db907d4 
>   src/personsmodel.cpp c2d97f6 
>   autotests/CMakeLists.txt 676f170 
>   autotests/fakecontactsource.h 5852297 
>   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 
>   CMakeLists.txt 3df5f0b 
> 
> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20150107/c08905e3/attachment-0001.html>