[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).

If you need help with this then I can try to mentor you.


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