Re: [evolution-patches] Free/Busy Bounty rev 2

[Rodrigo Moya <rodrigo ximian com>]

On Mon, 2004-01-12 at 11:34 -0700, Gary Ekker wrote:
> > 
> > > +                       icomp = e_cal_component_get_icalcomponent
> > (pub_comp);
> > > +                       icomp_clone =
> > e_cal_component_get_icalcomponent (*clone);
> > > +                       for (prop = icalcomponent_get_first_property
> > (icomp,
> > > +
> > ICAL_FREEBUSY_PROPERTY);
> > > +                               prop != NULL;
> > > +                               prop = icalcomponent_get_next_property
> > (icomp, 
> > > +
> > ICAL_FREEBUSY_PROPERTY))
> > > +                       {
> > > +                               icalproperty *p;
> > > +               
> > > +                               p = icalproperty_new_clone (prop);
> > > +                               icalcomponent_add_property
> > (icomp_clone, p);
> > > +                       }
> > >
> > what is this 'for' loop for? You are adding the properties from the
> > component you already cloned.
> > 
> 
> There are two components here. What this function does it aggregate the
> data. So if we are getting Free/Busy from multiple calendars, this is
> called once for each ESource and the FB properties are all added to one
> component. If there still seems to be a problem here given that
> information, let me know. I can't think of another way to accomplish
> this right now.
> 
oh, right, sorry, I was wrong. I thought icomp_clone was a clone of
icomp, which is not the case, and was thinking the for loop for adding
the properties was redundant. But you are right, it's ok.

> +       idle_id = g_new0 (guint, 1);
> +       *idle_id = g_idle_add ((GSourceFunc)
> init_calendar_publishing_cb, GINT_TO_POINTER (idle_id));
> 
there is no need to allocate 'idle_id'. Just use a 'int' and pass it to
init_calendar_pulishing_cb with GINT_TO_POINTER.

> +       real_uri = soup_uri_new(uri);
> +       if (!real_uri) {
> +               g_warning (G_STRLOC ": Invalid URL: %s", uri);
> +               return FALSE;
> +       }
> 
here, you have to g_object_unref the SoupSession created some lines
above.

> +       /* build the SOAP message */
> +       msg = soup_message_new_from_uri (SOUP_METHOD_PUT, real_uri);
> +       if (!msg) {
> +               g_warning (G_STRLOC ": Could not build SOAP message");
> +               return FALSE;
> +       }
the same here, when you return, you need to free the SoupSession, and
the SoupUri.

> +       /* send message to server */
> +       soup_session_send_message (session, msg);
> +       if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) {
> +               g_warning(G_STRLOC ": Could not publish Free/Busy: %d:
> %s", 
> +                         msg->status_code, 
> +                         soup_status_get_phrase(msg->status_code));
> +               return FALSE;
> +       }
ditto

Apart from that, and that it misses the new files (e-pub-publish.[ch]
and url-editor-dialog.[ch]), it looks ok. Please send those files and
we'll be set :-)

cheers