GNOME Bugzilla – Bug 678811
Fix eds backend unit tests
Last modified: 2012-06-25 22:05:50 UTC
Created attachment 217237 [details] [review]
edsunittests branch squashed into one patch.
Unit tests for the eds backend were disabled shortly before folks 0.7.1 release when porting to eds 3.5.3 api changes. They need to be re-enabled and fixed to work with the eds api changes.
The branch at http://git.gnome.org/browse/folks/log/?h=edsunittests fixes the eds backend unit tests. I've attached a patch for review that has the same changes as that branch.
Review of attachment 217237 [details] [review]:
A number of trivial style problems to fix, a comment to be added, and then this can be committed to master. Please squash a few of the commits together first though so that they’re all self-contained. Don’t forget to update the NEWS file and link to this bug report from the final commit! Thanks.
@@ +105,3 @@
this._addressbook.open_sync (false, null);
+ this._addressbook.get_source().get_uid ();
Missing space before the first ‘()’. Also, why is the UID being used for the address book name?
@@ +117,3 @@
public void set_as_default ()
Missing space before the ‘(’. There are more of them below, too.
@@ +135,2 @@
Please explicitly use ‘this.’ when referring to things in the same class.
@@ +267,1 @@
+ this._addressbook.remove.begin (null, (o, r) =>
There should probably be a comment here explaining that we’re using the async method because the sync method hangs, and linking to a bug number if one exists.
@@ +280,3 @@
+ GLib.warning ("Unable to remove addressbook %s because: %s\n",
+ this._addressbook_name, e.message);
The second line should be indented 4 spaces more than the first because it’s wrapped. Same with the other warning above.