[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-extra-gear
Subject:    Re: [Kde-extra-gear] [PATCH] Add support for multiple phone numbers
From:       Kevin Krammer <kevin.krammer () gmx ! at>
Date:       2010-01-04 8:05:04
Message-ID: 201001040905.15266.kevin.krammer () gmx ! at
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On Sunday, 2010-01-03, Stefano Avallone wrote:
> Hi list,
> 
> I implemented the support for multiple phone numbers and multiple email
> addresses in the akonadi googledata agent and libgcal. The type of a phone
> number and an email is also stored. Thanks to Kevin's suggestion, the
>  google group membership info for each contact is stored as a custom
>  property, and thus the membership is not lost upon a contact update made
>  within
> kaddressbook.
> 
> Please find attached the patches against:
> 
> svn://anonsvn.kde.org/home/kde/trunk/extragear/pim/googledata
> 
> and
> 
> git://repo.or.cz/libgcal.git
> 
> I have done some tests and it seems to work. Please review the patches and
> test it. It would be great if those patches could be applied, as they
>  provide a much needed functionality, in my opinion.

I've looked through the patch for the resource and it looks quite ok 
(obviously Adenilson has the last say on both patches).

I think the two helper functions should start with lower case characters 
because this is the most common style for methods and functions anywhere in 
KDE.

The transfer of pointer ownership when setting values is a bit strange, the 
values themselves are transferred, the container array is not.
Since the API docs of the libgcal function does not require any parameter to 
be dynamically allocated on the heap, I think it would be safer not to assign 
string pointers inside but to strdup() them and consequently let the caller 
keep ownership of the array and the strings.

Cheers,
Kevin

P.S.: for patches against KDE's repository I'd recommend doing a review 
request at reviewboard.kde.org
This simplifies the review process a lot as one can see the code surrounding 
the changes
-- 
Kevin Krammer, KDE developer, xdg-utils developer
KDE user support, developer mentoring

["signature.asc" (application/pgp-signature)]

_______________________________________________
Kde-extra-gear mailing list
Kde-extra-gear@kde.org
https://mail.kde.org/mailman/listinfo/kde-extra-gear


[prev in list] [next in list] [prev in thread] [next in thread]