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

[Sivaiah N <snallagatla novell com>]

On Mon, 2004-03-01 at 20:36, Rodrigo Moya wrote:
> 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_contaioner class is used not only for folders ,but now also for
books and in future may be for some other type of containers. So this
check is not needed here. is_writable field indicates whether an address
book is personal or system one. user can not  write into system address
book. i am reading this in e_gw_connection_get_address_book_list 

> 
> > -        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.
Actually the view should be passed as an argument instead of hard coding
here. "reciepents" view is specific to calendar.
> 
> > +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?
> 
Actually i am using e_gw_connection_remove_item. i will remove this
function it is not needed.

> cheers
> 
> _______________________________________________
> Evolution-patches mailing list
> Evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches