[Kde-pim] Review Request: Add new "default" fields to KPIMIdentities::Identity

[Ingo Klöcker]

> On March 14, 2012, 9:47 p.m., Ingo Klöcker wrote:
> > I have strong reservations against adding a kabc dependency to kpimidentities. This feels just wrong. But please read on ...
> > 
> > Your patch lacks any code actually saving and reading the added mDefaultAddressee on/from disk. Serializing a complete KABC::Addressee object in something as simple and light-weight as an Identity object feels wrong. OTOH, maybe your idea is not so bad after all. Maybe you just didn't think far enough ahead. Maybe the complete Identity class should be rethought in terms of KABC::Addressee, i.e. everything currently stored in the custom config format used by Identity could easily be stored in a KABC::Addressee object in special X-PIMIdentity-* properties. Thinking further ahead this would mean the PIM identities could be stored everywhere where we can store address books (via Akonadi). All of a sudden we could store the PIM identities in the cloud and share them between different computers. That'd be awesome. But it will require quite some work.
> > 
> > Having said this, I suggest you scrap your patch and start rewriting the storage of PIM identities in terms of KABC::Addressee, so that the Identity class becomes a simple convenience class around KABC::Addressee objects (with special PIM identity properties).
> 
> Ingo Klöcker wrote:
>     If you need help with this then I can try to mentor you.
> 
> Robert Mathias Marmorstein wrote:
>     I agree with all of this.  I can probably figure out how to serialize the mDefaultAddressee object, but it still "feels wrong" to go this route.  
>     
>     Your suggestion that we rewrite the Identity stuff in terms of Addressee sounds a lot like what we already have in the (deprecated) KABC::StdAddressBook class.  Perhaps the right solution is to simply unmark StdAddressBook as deprecated?  
>     
>     I'd love to learn more about how all this works.  Does anyone know why the new Identity stuff was split out of KABC?  What is the advantage of KPimIdentity over KABC::StdAddressBook?

kpimidentities was split out off KMail and KNode to unify their identity handling. It never had anything to do with KABC. Where did you read that StdAddressBook is deprecated in favor of KPimIdentity? Whereever you read this, it's nonsense.

From http://api.kde.org/4.x-api/kdepimlibs-apidocs/kabc/html/classKABC_1_1StdAddressBook.html:
Deprecated: Port to libakonadi-kontact. For instance using Akonadi::ContactSearchJob. See http://techbase.kde.org/Development/AkonadiPorting/AddressBook for details.

StdAddressBook is/was a convenience class for accessing your standard address book (containing information about your contacts) while KPimIdentity offers an API for managing several identities of yourself. Rewriting the Identity stuff in terms of Addressee means using Akonadi (probably with a special akonadi_vcarddir_resource for a special address book holding your identities). StdAddressBook is dead because it offers a synchronous API which simply doesn't work with Akonadi's asynchronous design.


- Ingo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104181/#review11417
-----------------------------------------------------------


On March 14, 2012, 4:54 p.m., Robert Mathias Marmorstein wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104181/
> -----------------------------------------------------------
> 
> (Updated March 14, 2012, 4:54 p.m.)
> 
> 
> Review request for KDEPIM and KDEPIM-Libraries.
> 
> 
> Description
> -------
> 
> The KABC::StdAddressBook class has been deprecated and KPIMIdentities is the suggested replacement.  Unfortunately, many of the fields that were available in StdAddressBook are missing in the Identity class.  It's possible to use the property() and setProperty() methods to store custom fields, but that means that different apps might use different field names for things like home phone number and locality (city).  
> 
> We use this functionality in KOffice to store information about the document author.  Internally, we can just use the property methods, but it would be nice to have interoperability with other applications (such as KAddressBook).  
> 
> This patch adds methods for accessing some of the missing fields (the ones we use in KOffice).  Some of the functionality of StdAddressBook is still missing -- instead of a list of phone numbers, I hardwired in a "Home" and "Work" phone number -- but it is a step forward.
> 
> This is code I am not that familiar with, but it seems pretty straightforward.  I welcome any comments/constructive criticism.
> 
> 
> Diffs
> -----
> 
>   kalarmcal/CMakeLists.txt 0c0e956 
>   kpimidentities/CMakeLists.txt 7b1569d 
>   kpimidentities/identity.h b0d0e7c 
>   kpimidentities/identity.cpp e1023b3 
> 
> Diff: http://git.reviewboard.kde.org/r/104181/diff/
> 
> 
> Testing
> -------
> 
> kdepimlibs compiles and passes the same unit tests it did before the change.  Unfortunately, I just switched my main development computer to Kubuntu from Archlinux (which made testing library changes very easy) and am still trying to figure out the best way to test library changes without clobbering the system packages.  Any suggestions/help would be much appreciated.
> 
> 
> Thanks,
> 
> Robert Mathias Marmorstein
> 
>

_______________________________________________
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/