On Mon, Apr 18, 2011 at 10:18 PM, Matthew Barnes <mbar...@redhat.com> wrote: > On Mon, 2011-04-18 at 15:57 +0200, Milan Crha wrote: >> I just created a new branch 'eclient' in eds [1] where is added my work >> on the new API which will deprecate EBook/ECal. It is following glib/gio >> async pattern and, I believe, makes things more coherent. > > Thanks for posting this, Milan. I want to emphasize how important these > new APIs are and ask everyone to look it over and provide comments. > > I had a sneak peek at this over the weekend and jotted some notes. So > far I've only reviewed in detail the client-side headers because that's > what I'm most concerned about getting right. The rest of it can be > tweaked and changed as needed -- even the backend APIs are never truly > frozen. But the client-side APIs *will* be frozen, since that's what > other projects will be migrating to and attempting to use. > > The new client-side APIs are: > > EClient (abstract base class) > http://git.gnome.org/browse/evolution-data-server/tree/libedataserver/e-client.h?h=eclient > > ECalClient (replaces ECal) > http://git.gnome.org/browse/evolution-data-server/tree/calendar/libecal/e-cal-client.h?h=eclient I would like you to a incorporate some change to the free/busy api. Some servers allow querying free/busy information for multiple users at a time and the results appear in a iterative fashion. The freebusy information of some users are delivered first, while the server keeps fetching other user's free/busy information. So if we could have he FreeBusy api such as this, we could leverage those features,
ECalClientFreeBusy e_cal_client_get_freebusy_object_sync (ECalClient *client, time_t start, time_t end, const GSList *users, GCancellable *cancellable, GError **error); (with a async counter-part) Signal: freebusy_received (ECalClientFreeBusy *fbobject, const gchar *user, GSList *ecalcomps) The clients could watch for the signal and update the freebusy information as and when they receive from eds. - Chenthill. > > EBookClient (replaces EBook) > http://git.gnome.org/browse/evolution-data-server/tree/addressbook/libebook/e-book-client.h?h=eclient > > ECredentials (authenication) > http://git.gnome.org/browse/evolution-data-server/tree/libedataserver/e-credentials.h?h=eclient > > > I'll split my comments into two posts so this doesn't get too > overwhelming. Simple, hopefully non-controversial stuff here and > meatier topics in a separate post. Overall I'm pretty happy with > the APIs. > > > * There's some overlap between the new EClient API and the new > ESource API that I'm working on. Some functions will need to be > dropped once the new ESource API is in place, so I don't know if > you want to do this now or wait. > > ESourceList is being removed so obviously any functions involving > ESourceList will be dropped: > > e_client_util_get_system_source() > e_client_util_set_default() > e_client_util_get_source_for_uri() > e_cal_client_get_sources() > e_book_client_get_sources() > > We will no longer refer ESources using URIs. We will only refer > to ESources by their unique ID string. So the following functions > will be dropped: > > e_client_get_uri() > e_cal_client_new_from_uri() > e_book_client_new_from_uri() > > Default sources will be tracked using the new ESourceRegistry API, > so the following functions will be dropped: > > e_cal_client_set_default() > e_cal_client_set_default_source() > e_book_client_set_default() > e_book_client_set_default_source() > > There's a few functions that I think are too trivial to keep in light > of the lookup capabilities of ESourceRegistry. I'd like to keep the > EClient APIs as slim as possible initially and drop these functions: > > e_cal_client_new_system() > > Instead do: > > source = e_source_registry_lookup_by_uid (registry, "system"); > client = e_cal_client_new (source, source_type, error); > > e_cal_client_new_default() > > Instead do: > > source = e_source_registry_get_default_calendar (registry); > client = e_cal_client_new (source, E_CAL_SOURCE_TYPE_EVENT, error); > > (similar functions exist for tasks and memos) > > e_book_client_new_system_addressbook() > > Instead do: > > source = e_source_registry_lookup_by_uid (registry, "system"); > client = e_book_client_new (source, error); > > e_book_client_new_default_addressbook() > > Instead do: > > source = e_source_registry_get_default_address_book (registry); > client = e_book_client_new (source, error); > > > * We should use GIO error codes whenever possible, and I see quite a bit > of overlap here. I think following error codes should be dropped: > > E_CAL_CLIENT_ERROR_SUCCESS > E_BOOK_CLIENT_ERROR_SUCCESS > > There's no need for an error code for successful operations. > > E_CAL_CLIENT_ERROR_INVALID_ARG > E_BOOK_CLIENT_ERROR_INVALID_ARG > > Use G_IO_ERROR_INVALID_ARGUMENT. > > E_CAL_CLIENT_ERROR_BUSY > E_BOOK_CLIENT_ERROR_BUSY > > Use G_IO_ERROR_BUSY. > > E_CAL_CLIENT_ERROR_PERMISSION_DENIED > E_BOOK_CLIENT_ERROR_PERMISSION_DENIED > > Use G_IO_ERROR_PERMISSION_DENIED. > > E_CAL_CLIENT_ERROR_NOT_SUPPORTED > E_CAL_CLIENT_ERROR_PROTOCOL_NOT_SUPPORTED > E_BOOK_CLIENT_ERROR_NOT_SUPPORTED > E_BOOK_CLIENT_ERROR_PROTOCOL_NOT_SUPPORTED > > Use G_IO_ERROR_NOT_SUPPORTED. > > E_CAL_CLIENT_ERROR_CANCELLED > E_BOOK_CLIENT_ERROR_CANCELLED > > Use G_IO_ERROR_CANCELLED. > > E_CAL_CLIENT_ERROR_INVALID_SERVER_VERSION > E_BOOK_CLIENT_ERROR_INVALID_SERVER_VERSION > > I don't think these are necessary anymore since we started > using versioned bus names for the addressbook and calendar > services. If there's a client/server version mismatch, > the client will get a D-Bus error about it. > > E_CAL_CLIENT_ERROR_DBUS_ERROR > E_BOOK_CLIENT_ERROR_DBUS_ERROR > > I expect GDBus will handle D-Bus errors and hand us back a > G_IO_ERROR_DBUS_ERROR. > > E_BOOK_CLIENT_ERROR_NO_SPACE > > Use G_IO_ERROR_NO_SPACE. > > E_CAL_CLIENT_ERROR_OTHER_ERROR > E_BOOK_CLIENT_ERROR_OTHER_ERROR > > Use G_IO_ERROR_FAILED. > > > * Of the remaining error codes, a number of them in ECalClient and > EBookClient can be combined and moved to EClient. Especially ones > related to offline and authentication, since that's what EClient > handles. > > > * I would really prefer we use GList instead of GSList throughout the > API. GSList is silly and really should never have been added to GLib, > in my opinion. Most modern GNOME APIs that I've seen prefer GList, > and it's a pain in the butt having to convert between the two. > > > * I thought backends could send their own custom error messages now, so > are e_cal_client_error_to_string() and e_book_client_error_to_string() > still necessary? > > > * In EBookClient, drop the 'const' qualifier from EContact arguments. > Trying to use 'const' with GObjects is misguided and pointless. I've > cursed at EIterator many times for this. > > > * Why do we have "get_capabilities" functions in EClient, ECalClient and > EBookClient? Are they different sets of capabilities? And if getting > capabilities involves a D-Bus call then doesn't it need to be async > everywhere? > > > That's all I have for now. I'll save the heavier stuff for a separate > post. > > _______________________________________________ > evolution-hackers mailing list > evolution-hackers@gnome.org > To change your list options or unsubscribe, visit ... > http://mail.gnome.org/mailman/listinfo/evolution-hackers > _______________________________________________ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers