Re: Re: [Planner Dev] Add "Assigned to" column to the tasks display

[Richard Hult <richard imendio com>]

On ons, 2004-07-21 at 10:14 -0400, wrote:
> > 
> > From: Richard Hult <richard imendio com>
> > Date: 2004/07/21 Wed AM 04:02:44 EDT
> > To: Planner Dev <planner-dev lists imendio com>
> > Subject: Re: [Planner Dev] Add "Assigned to" column to the tasks display
> > 
> > On mån, 2004-07-19 at 21:04 -0400, Chris Morgan wrote:
> > > Just wanted to thank the developers of planner for an excellent and very 
> > > useful program.
> > > 
> > > I wrote up this patch to add a column to the task view with the resources a 
> > > task is assigned to.  This seemed quite useful to have and allows for knowing 
> > > which tasks have no one assigned to at the moment.  This is my first planner 
> > > patch so please tell me what things I need to clean up to get this into cvs.
> > 
> > Hi,
> > 
> > The patch looks like a useful addition! I have a few comments on the
> > code, mostly style issues. We try to use a very consistent style to make
> > the code easy to read and maintain:
> > 
> > The indentation seems to be off in a few places, e.g.:
> > 
> > +       gchar  *assigned_to;
> > +       GList  *resources;
> > +       MrpAssignment  *assignment;
> > +       MrpResource    *resource;
> > 
> > Those should be aligned.
> > 
> 
> Of course, I'll take care of that and the rest of the formatting issues.
> 
> 
> > +       resources = mrp_task_get_assigned_resources(task);
> > +
> > +        for (l = resources; l; l = l->next) {
> > 
> > Here as well.
> > 
> > +               if(assigned_to)
> > +               {
> > 
> > The brace should be on the same line as the if, same with else 
> > (} else {).
> > 
> > newstr = g_strdup_printf("%s, %s", assigned_to, name);
> > 
> > We use spaces before "(".
> > 
> > Other than that, the patch looks fine. I have one objection should, that
> > is that we should really cache the string instead of building if in the
> > data_func. data_func is run every time the cell is redrawn, like when
> > you scroll or move the mouse over it.
> > 
> 
> Are there other data functions in that code that cache their values rather than retrieve them each time?  Is that the purpose for the task tree view?  

The WBS stuff is cached, you could look at that (search for "wbs" in the
same file).

The purpose of the separate file with the task tree is mostly to be able
to share that code between the task view and the gantt view. 

/Richard

-- 
Imendio HB, http://www.imendio.com/