From kde-extra-gear  Mon Jan 04 08:05:04 2010
From: Kevin Krammer <kevin.krammer () gmx ! at>
Date: Mon, 04 Jan 2010 08:05:04 +0000
To: kde-extra-gear
Subject: Re: [Kde-extra-gear] [PATCH] Add support for multiple phone numbers
Message-Id: <201001040905.15266.kevin.krammer () gmx ! at>
X-MARC-Message: https://marc.info/?l=kde-extra-gear&m=126259238410375
MIME-Version: 1
Content-Type: multipart/mixed; boundary="--===============0097822386=="

--===============0097822386==
Content-Type: multipart/signed;
  boundary="nextPart1657469.8KSyLHs0Xz";
  protocol="application/pgp-signature";
  micalg=pgp-sha1
Content-Transfer-Encoding: 7bit

--nextPart1657469.8KSyLHs0Xz
Content-Type: Text/Plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: quoted-printable

On Sunday, 2010-01-03, Stefano Avallone wrote:
> Hi list,
>=20
> 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.
>=20
> Please find attached the patches against:
>=20
> svn://anonsvn.kde.org/home/kde/trunk/extragear/pim/googledata
>=20
> and
>=20
> git://repo.or.cz/libgcal.git
>=20
> 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=20
(obviously Adenilson has the last say on both patches).

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

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

Cheers,
Kevin

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

--nextPart1657469.8KSyLHs0Xz
Content-Type: application/pgp-signature; name=signature.asc 
Content-Description: This is a digitally signed message part.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iD8DBQBLQaExnKMhG6pzZJIRAt8vAKCEbMZ9QzpXsFv8oZG1xABxFedT4wCdG5O8
898FsokW/o86WD+knah2zB4=
=tOj1
-----END PGP SIGNATURE-----

--nextPart1657469.8KSyLHs0Xz--

--===============0097822386==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

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

--===============0097822386==--