Re: [evolution-patches] patch for bug #60463 calendar



On Wed, 2004-06-30 at 11:58, Rodrigo Moya wrote:
[...]
> > THings like
> > 
> > -  itt = icaltime_from_timet (t, 0);
> > +  if (default_zone)
> > +     itt = icaltime_from_timet_with_zone (t, 0, default_zone);
> > +  else
> > +     itt = icaltime_from_timet_with_zone (t, 0, 0);
> >    ipt.start = itt;
> >  
> > 
> > Could be rewritten without making 2 calls to the same method.
> > 
> no, if you send a NULL timezone to icaltime_from_timet_with_zone, it
> will probably crash. As I said in my previous mail, you should be using
> icaltime_from_timet when there's no timezone.

I didn't see your previous mail?! Can't find it in my archive.


On Thu, 2004-07-01 at 07:09, JP Rosevear wrote: 
[...]
> Missing the point - rodrigo is saying you have to call a different
> function if default_zone == NULL, so you can't combine the statements.

OK. But then the initial patch is buggy as it uses the same method calls
on each branch of the if statements. That's why I wanted to combine
them. Maybe Rodrigo addressed that in his review, but I didn't see it.

So never mind my comments, but as chenthill made a new version of the
patch using my review, I guess he/she has to rework it with Rodrigo's
review in mind.

J





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]