Re: [evolution-patches] patch for groupwise address book backend

[Rodrigo Moya <rodrigo ximian com>]

On Sun, 2004-02-22 at 19:24 +0530, Sivaiah N wrote:

> Hi, 
> I am attaching patch for first version of the groupwise address book
> backend.i have attached two diff files.
> groupwise_addressbook_backend.diff contains the new files
> e_book_backend_groupwise.h and e_book_backend_groupwise.c 
> 
> groupwise_common_classes.diff contains the changes i made to common
> groupwise classes e-gw-* (present under
> evolution-data-server/servers/groupwise in groupwise-addressbook-backend
> branch of e-d-s) to implement apis required for address book backend.
> 
some comments on this part:

> +void 
> +e_gw_message_write_string_parameter_with_attribute (SoupSoapMessage
> *msg, const char *name, const char *prefix, const char *value, 
> +                                                   char
> *attribute_name, char*attribute_value)
> 
this would be better done, IMO, as:

...write_string_parameter_with_attrbutes (msg, name, prefix, value, ...)

so that we can use a list of name/value pairs for 'n' attributes.

> -       if (strcmp (soup_soap_parameter_get_name (param), "folder") !=
> 0) {
> -               g_warning (G_STRLOC ": SOAP parameter is not a
> folder");
> -               return FALSE;
> -       }
what is the reason for removing those lines? Also, it seems you are not
reading the is_writable field from the SOAP parameter, or am I wrong?

> -        e_gw_message_write_string_parameter (msg, "view", NULL,
> "recipients");
> +       //        e_gw_message_write_string_parameter (msg, "view",
> NULL, "recipients");
> 
if that code is not going to be used, it's better to remove it.

> +EGwConnectionStatus 
> +e_gw_connection_remove_book (EGwConnection *cnc, char *book_uid)
> +{
> +       SoupSoapMessage *msg;
> +       int status;
> +       SoupSoapResponse *response;
> +
> +       msg = e_gw_message_new_with_header (cnc->priv->uri, cnc->priv-
> >session_id, "removeItemRequest");
> +       e_gw_message_write_string_parameter (msg, "id", NULL,
> book_uid);
> +       e_gw_message_write_footer (msg);
> 
This should probably just call e_gw_connection_remove_item, shouldn't
it?

cheers