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 697262 - uses deprecated e-d-s functions
uses deprecated e-d-s functions
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: e-d-s backend
git master
Other Linux
: Normal normal
: Unset
Assigned To: folks-maint
folks-maint
Depends on:
Blocks:
 
 
Reported: 2013-04-04 12:53 UTC by Simon McVittie
Modified: 2013-09-26 18:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Xavier's fix to stop using the deprecated EDS API (7.28 KB, patch)
2013-09-26 01:11 UTC, Travis Reitter
needs-work Details | Review

Description Simon McVittie 2013-04-04 12:53:05 UTC
edsf-persona-store.vala:760.35-760.64: warning: E.BookClient.new has been deprecated since 3.8
edsf-persona-store.vala:1081.17-1081.44: warning: E.Client.open has been deprecated since 3.8
edsf-persona-store.vala:1136.15-1136.47: warning: E.Client.is_opened has been deprecated since 3.8

This results in Folks needing --disable-fatal-warnings.

We already depend on e-d-s 3.8, so we might as well do things the modern way.

(I considered using E.BookClient.connect_direct_sync(), but my testing indicates that this is around 10% slower than the current code, so we should do the equivalent of the current code without using deprecated API.)
Comment 1 Travis Reitter 2013-09-26 01:11:32 UTC
Created attachment 255787 [details] [review]
Xavier's fix to stop using the deprecated EDS API

From branch:

http://cgit.collabora.com/git/user/xclaesse/folks.git/log/?h=deprecated
Comment 2 Travis Reitter 2013-09-26 17:12:32 UTC
Review of attachment 255787 [details] [review]:

Mostly looks good; please make the following minor corrections and apply:

               /* Connect to the address book. */
               this._addressbook = yield E.BookClient.connect (this.source, null);

               ((!) this._addressbook).notify["readonly"].connect (
                   this._address_book_notify_read_only_cb);
 
-              yield this._open_address_book ();
               debug ("Successfully finished opening address book %p for " +
                   "persona store ‘%s’ (%p).", this._addressbook, this.id, this);

Please update this comment to say "Connect and open the address book" so it's clear that the opening happens as a side-effect.

The call to connect() should also be wrapped in a try {} catch {} since it could throw an error if it fails. The function _open_address_book() did some error handling (though I guess we never could have hit it?) so we should make sure that it's in place for the connect() call (it should have been in the first place).


And please add an entry to the NEWS for this bug fix.
Comment 3 Xavier Claessens 2013-09-26 18:45:35 UTC
It was already inside a try{} block. I pushed the patch with the fix in the comment, and updated NEWS. Thanks.