Re: scaffold & gnome-build patch

[aurelien naldi <aurelien resus univ-mrs fr>]

On jeu, 2004-01-15 at 00:30 -0300, Gustavo Giráldez wrote: 
> Hi Aurelien,

Hi,

> On Tue, 2004-01-13 at 14:53, aurelien naldi wrote:
> > On mar, 2004-01-13 at 01:46 -0500, John (J5) Palmieri wrote:
> > > Sorry, got caught up with work.  I just applied your Scaffold patch and
> > > it didn't cleanly patch the latest CVS sources.  Can you make another
> > > patch?  I will check out your gnome-build patch tomorow.
> > 
> > the gnome-build one wasn't correct as well, so here they are
> > 
> > I hope those one are OK :)
> 
> As before, I'll comment only on the gnome-build patch.  Looks good in
> general (i.e. the approach is right) but I've got some comments:

thanks for your comments :)

[..]


> Here it would be interesting to add some prefix to the id to indicate
> it's a project target being built.  Otherwise you couldn't tell the
> difference between the special "configure" target and a "configure"
> program.

I had the same feeling, but the id contain the path to the program and
it would be SO durty (but not incorrect) to create a program at the root
of the project, anyway I've just addes a "USER:" before those targets.

> > g_strdup_printf("Build specific > target: %s", node->name);
> 
> This string needs to be marked up for translation.

Not sure what has to be done for that, is this more correct ?

 g_strdup_printf ("%s: %s", _("Build specific target"), node->name);


> > @@ -333,6 +333,7 @@
> >                         gtk_tree_model_get (model, &iter2,
> >                                             GBF_PROJECT_MODEL_COLUMN_DATA, &data,
> >                                             -1);
> > +                       iter = iter2;
> 
> Doh :-)  Nice catch.
not so hard to find: I experienced an infinite loop when adding a new
target while I wasn't on the good node...


> The glue-factory.c and glue-plugin.c hunks are already committed (I
> fixed the warnings yesterday).
nice, thanks :)


> Minor nit pick: Please leave a space between the opening parenthesis and
> the last character.  I notice you don't do that in some places.
Oops, I'm not used to this, this should be corrected in those new patch
and I'll take care now...


> Thanks for your work.
> 
> Regards,
> Gustavo


so here is a new version of the gnome-build patch and also a little change to the scaffold one.
I'm quite happy with them now...

another question: I supose scaffold will move to the new API for gtk
menu/toolbar, does it need a big rewrite or is it pretty trivial ??

problem remining with the build menu: they are never updated when the
project is closed or a target removed/added.

-- 
aurelien