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/