After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 655917 - Rebase EmailDetails.email_addresses upon an AbstractFieldDetails-derived class
Rebase EmailDetails.email_addresses upon an AbstractFieldDetails-derived class
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: libfolks
git master
Other Linux
: Normal normal
: folks-0.6.0
Assigned To: folks-maint
folks-maint
Depends on:
Blocks: 653684 655911
 
 
Reported: 2011-08-03 17:30 UTC by Travis Reitter
Modified: 2011-08-12 16:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add EmailFieldDetails (16.51 KB, patch)
2011-08-09 14:28 UTC, Travis Reitter
reviewed Details | Review

Description Travis Reitter 2011-08-03 17:30:24 UTC
Proposed API changes:
-  public abstract Set<FieldDetails> email_addresses { get; set; }
+  public abstract Set<EmailFieldDetails> email_addresses { get; set; }
Comment 1 Travis Reitter 2011-08-09 14:28:47 UTC
Created attachment 193489 [details] [review]
Add EmailFieldDetails

Patch from branch:

http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=bgo655917-email-field-details
Comment 2 Philip Withnall 2011-08-10 09:10:43 UTC
Review of attachment 193489 [details] [review]:

A few trivial missing comments, and a type issue to think about (but which we will probably end up not being able to change).

::: NEWS
@@ +36,3 @@
+* Bug 655917 — Rebase EmailDetails.email_addresses upon an
+  AbstractFieldDetails-derived class
+* Bug 655374 — Un-break avatar tests

Does this really fix bug #655374.

::: backends/eds/lib/edsf-persona-store.vala
@@ +865,2 @@
   private async void _set_contact_attributes (E.Contact contact,
+      Set<AbstractFieldDetails<string>> new_attributes,

Is it possible to create generic methods in Vala (without making the containing class a generic class)? If so, I think it would be tidier to make _set_contact_attributes() generic, and then change the type of new_attributes to Set<AbstractFieldDetails<T>> instead.

::: backends/tracker/lib/trf-persona-store.vala
@@ +859,3 @@
   private async void _build_update_query_set (
       Tracker.Sparql.Builder builder,
+      Set<AbstractFieldDetails<string>> properties,

Same comment as for EDS' _set_contact_attributes().

@@ +1951,2 @@
   internal async void _set_unique_attrib_set (Folks.Persona persona,
+      Set<AbstractFieldDetails<string>> properties, Trf.Attrib attrib)

Same comment as for EDS' _set_contact_attributes().

::: backends/tracker/lib/trf-persona.vala
@@ +108,2 @@
   /**
    * {@inheritDoc}

Needs a @since UNRELEASED comment.

::: folks/individual.vala
@@ +357,2 @@
   /**
    * {@inheritDoc}

Needs a @since UNRELEASED.
Comment 3 Travis Reitter 2011-08-12 16:00:50 UTC
commit 9388c1eb0c71735bf750a03248d5f38c96681fd9
Author: Travis Reitter <travis.reitter@collabora.co.uk>
Date:   Wed Aug 3 13:49:32 2011 -0700

    Rebase EmailDetails.email_addresses upon EmailFieldDetails
    
    Closes: bgo#655917 - Rebase EmailDetails.email_addresses upon an
    AbstractFieldDetails-derived class