[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] For Review: libkcal/calendar.h cleaning
From:       Allen Winter <winter () kde ! org>
Date:       2005-03-04 16:58:36
Message-ID: 200503041158.37006.winter () kde ! org
[Download RAW message or body]

On Friday 04 March 2005 11:29 am, Reinhold Kainhofer wrote:
> On Friday 04 March 2005 16:59, Allen Winter wrote:
> > > > //TODO: should there be rawIncidencesForDate() for the sake of
> > > > consistency?
> >
> > No comment about a new rawIncidencesForDate() method?
> 
> I haven't thought about this at all.
> 
OK. I don't think rawIncidencesForDate() is needed at this time.
Sorta weird mixing Event start dates with To-do start->due date range with Journal date.
Forget this idea, I guess.

> > > >     QStringList incidenceCategories();
> > >
> > > This method should probably be renamed. I just couldn't come up with a
> > > better name when I added it.
> >
> > What's wrong with simply categories()?
> 
> This is the list of all categories used by at least one incidence in the 
> calendar. That's not the list of categories that is available in KOrganizer's 
> UI (but this list is merged into KOrganizer's list, so KOrganizer always has 
> at least all used categories). For that reason a more complicated name was 
> chosen.
> 
So KOrganizer has a list of all possible categories (including ones defined by the user).
And Calendar::incidenceCategories() has a list of only the categories used by this Calendar.
I don't see the problem naming the method categories().  KOrganizer has its own list
somewhere by itself.   Calendar::categories() seems self-explanatory.  I must be confused.

> > > >     /**
> > > >      * Return a filtered list of all Todos which are due on the
> > > > specifed date. * @param date request filtered Todos due on this QDate
> > > > only. * @return the list of filtered Todos due on the specified date.
> > > > */
> > > >     //TODO: rename this to todosForDueDate?
> > > >     //TODO: for consistency, do we need todosForStartDate?
> > >
> > > Or do we need a method that returns a to-do all "active" to-dos on that
> > > date (i.e. to-dos where startDate <= date <= dueDate)?
> >
> > I'll see how todos( const QDate &date) method is used by applications and
> > get back with a recommendation.
> 
> The thing is that there are wishes that to-dos are shown from their start time 
> until their end time. For this we would need such a method.
> 
OK.  I'll try to re-write todos( QDate ) that considers the startDate and the dueDate as
you describe above.  Actually, rawTodosForDate( QDate) ...

But do we also need a todos..(QDate) method that considers start date only? or due date only?

> > > >     /**
> > > >      * Remove all Relations from an Incidence.
> > > >      * @param incidence is a pointer to the Incidence to have a
> > > >      * Relation removed.
> > > >      */
> > > > //TODO: return bool instead of void?
> > > > //TODO: for consistency rename to deleteRelations?
> > >
> > > deleteRelations sounds so destructive...
> >
> > true..
> > I'll see if removeRelations() should return a bool instead of a void.
> 
> Why? Isn't it just an internal method to build up the hierarchy?
>
Yeah, forget making removeRelations() into a bool.
 
> > > >     QString mTimeZoneId;
> > > >
> > > >     int mTimeZone;         // timezone OFFSET from GMT (MINUTES)
> > >
> > > This variable doesn't make much sense anyway. The offset from GMT is
> > > never fixed (dst vs. winter time).
> >
> > OK, I'll try to eliminate mTimeZone.  I can't see that it is used for
> > anything in libkcal anyway.
> 
> Yep, doens't seem to be used.
> 
Check.

> > As you all know, changes I make in calendar.h will impact many other source
> > files in libkcal. I really would like a "go ahead" signal from the
> > maintainers before proceeding with this pretty large task.
> 
> I don't see any problems. The time zone handling needs to be cleaned up sooner 
> or later (in particular, we need to switch to the new libical version, and 
> then each incidence can have its own tz assigned).
> 
All right then.  Here I go...

-- 
Let's Keep the Political Talk Out of KDE PLEASE
_______________________________________________
kde-pim mailing list
kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
kde-pim home page at http://pim.kde.org/

[prev in list] [next in list] [prev in thread] [next in thread]