[webstack-discuss] [sfwnv-discuss] Code Review request for 6693315

[Bjorn Munch]

On 10/09 09.25, Sunanda Menon wrote:
> On 09/09/08 16:43, Bjorn Munch wrote:
> >On 09/09 14.40, Sunanda Menon wrote:
> >  
> >>Hi ,
> >>
> >>Please review the code changes for 6693315: bump mysql from 5.0.45 to 
> >>latest 5.0.*  at
> >>http://cr.opensolaris.org/~sunandam/6693315/ and let me know your 
> >>comments ASAP.
> >>    
> >
> >I have a few comments:
> >
> >install-sfw and install-sfw-64:
> >
> >  In the function fix_sed_path, why do you have a for loop over a list
> >  with only one element?
> >
> >  I also wonder about the name of that function, why "sed"?
> >
> >  
> The function is called fix_sed_path as I'm using sed to change the 
> internal path names being used in mysql_config.

Well, actually you were using ed, not sed. :-)  Anyway, IMHO the
function should be named for *what* it does, not *how*, I read the
name as meaning it fixed some "sed path" and that didn't make sense to
me.  How about "fix_ldpath"?

Oh, and if the for loop really does only one iteration, I suggest you
drop the loop.

> >*.cnf.patch:
> >
> >  Since these all do the same change, it may be better to concatenate
> >  them into a single patch file, then use the more common command line
> >  form "gpatch < my-cnf.patch".
> >  
> 
> Yeah ,I can try using one single patch but would prefer to keep it in 
> the Makefile and do the patching before I build the source .

OK!

-- 
Bjorn Munch                 Sun Microsystems
Trondheim, Norway       http://sun.com/postgresql/