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 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