Created attachment 195566 [details] [review]
patch

Review of attachment 195566 [details] [review]:

A few minor things which need improving.

::: backends/eds/lib/edsf-persona-store.vala
@@ +362,3 @@
+          else if (k == Folks.PersonaStore.detail_key (PersonaDetail.BIRTHDAY))
+            {
+              var birthday = (DateTime) v.get_boxed ();

This should be “DateTime?”, and you should make sure the code handles nulls gracefully.

::: backends/eds/lib/edsf-persona.vala
@@ +551,2 @@
   /**
+   * {@inheritDoc}

Needs a @since line.

@@ +553,3 @@
+   */
+  [CCode (notify = false)]
+  public string? calendar_event_id { get; private set; }

Using “private set” doesn't work, since the property was declared as settable.

The code should be the following instead:
    [CCode (notify = false)]
    public string? calendar_event_id
      {
        get { return null; } /* unsupported */
        set { this.change_calendar_event_id.begin (value); } /* not writeable */
      }

Since you haven't implemented change_calendar_event_id(), the setter will end up calling the default implementation from BirthdayDetails, which does nothing but throw an error.

Is there no way to get the calendar event ID from EDS?

@@ +790,3 @@
+  private void _update_birthday ()
+    {
+      E.ContactDate? bday = (E.ContactDate) this._get_property ("birth_date");

The cast should be nullable.

@@ +794,3 @@
+      if (bday != null)
+        {
+          DateTime d = new DateTime.utc ((int) bday.year, (int) bday.month,

You could use “var” here.

Does EDS guarantee to provide the birth date/time in UTC, or do we need to be doing a time zone conversion? Either way, there should probably be a brief comment about it here.

::: tests/eds/add-persona.vala
@@ +235,3 @@
 
+      Value? v9 = Value (typeof (DateTime));
+      DateTime dobj = new  DateTime.utc (1980, 1, 1, 0, 0, 0.0);

These should have type “DateTime?”.

@@ +413,3 @@
+      if (i.birthday != null)
+        {
+          DateTime dobj = new  DateTime.utc (1980, 1, 1, 0, 0, 0.0);

You could use “var” here.

::: tests/eds/set-birthday.vala
@@ +63,3 @@
+
+      v = Value (typeof (string));
+      v.set_string ("John McClane");

That's my name. Get your own.

Created attachment 195571 [details] [review]
updated patch

Review of attachment 195571 [details] [review]:

Ship it!

::: tests/eds/set-birthday.vala
@@ +110,3 @@
+          var name = (Folks.NameDetails) i;
+
+          if (name.full_name == "John McClane")

I protest that you're stealing my comedy-name-for-use-in-unit-tests.

Merged:

commit 45eb4c58a231aedcea1dd28cd305254bbdcc7233
Author: Raul Gutierrez Segales <rgs@collabora.co.uk>
Date:   Sat Sep 3 13:58:31 2011 +0100

    e-d-s: add test for setting birthday on an e-d-s Persona
    
    Helps: https://bugzilla.gnome.org/show_bug.cgi?id=657971

commit 53b36cddfe3b6e7b73b344c464f2a99449d9eb97
Author: Raul Gutierrez Segales <rgs@collabora.co.uk>
Date:   Sat Sep 3 13:49:58 2011 +0100

    e-d-s: extend add-persona test to include the BirthdayDetails interface

commit f7bbe3723cde7ba6a7ac91735a082366c9f52e70
Author: Raul Gutierrez Segales <rgs@collabora.co.uk>
Date:   Sat Sep 3 13:37:50 2011 +0100

    e-d-s: implement BirthdayDetails
    
    Fixes: https://bugzilla.gnome.org/show_bug.cgi?id=657971

commit 0aa6c4815ffa6171f939b6fb419bd0ba184bbbca
Author: Raul Gutierrez Segales <rgs@collabora.co.uk>
Date:   Sat Sep 3 15:13:22 2011 +0100

    Documentation: state the birthday's DateTime objects are UTC