Thanks Mukhta, good catch, updated. Regards, Suresh
Muktha Narayan wrote: > Hi Suresh, > > In prototype_com file, the 'group' entry for usr/share should be 'sys' > and not 'bin'. Please check. > Rest of the changes look fine. > > Regards > Muktha > > Suresh Chandrasekharan wrote: >> Pl. see the changed webrev modified as per the feedback at >> http://cr.opensolaris.org/~suresh/convmv_webrev/ >> >> Following comment is not implemented due to, the convnv binary is just a >> perl script and consequently _install E option cannot be used. I also >> have maintained the for loop, it does not do any harm >> >> d) Since there is only one binary and one man page that is being >> installed. In my opinion, the 'for' loops are not required e) >> The binary should be installed as executable 'E' and not as normal >> files as below: >> _install E ${i} ${BINDIR}/${i} 555 >> 'N' is generally used for normal files. >> >> >> Suresh Chandrasekharan wrote: >> >>> Thanks Paul. These will be added. >>> Regards, >>> Suresh >>> >>> Paul Cunningham wrote: >>> >>> >>>> Suresh, >>>> >>>> Here are a few more comments ... >>>> >>>> 1. usr/src/pkgdefs/SUNWconvmv/pkginfo.tmpl >>>> Add version number to the end of the DESC= string, eg. >>>> DESC="............. (1.14)" >>>> >>>> 2. usr/src/pkgdefs/SUNWconvmv/prototype_sparc >>>> & usr/src/pkgdefs/SUNWconvmv/prototype_i386 >>>> Change the Copyright statement lines to the current >>>> standard form, ie. as in prototype_com >>>> >>>> 3. usr/src/pkgdefs/SUNWconvmv/copyright >>>> This might need the pkg originator copyright statements >>>> added and the Sun disclaimer (ask James.Walker at Sun.COM, see >>>> http://cr.opensolaris.org/~martina/gdbm/usr/src/pkgdefs/SUNWgnu-dbm/copyright.html) >>>> >>>> >>>> >>>> Paul >>>> >>>> Steven M. Christensen wrote: >>>> >>>> >>>>> One more comment about #3 as in #4d, do you really need the patch >>>>> loop. I see only one patch. >>>>> >>>>> Steve C. >>>>> >>>>> >>>>> Muktha Narayan wrote: >>>>> >>>>> >>>>>> Hi Suresh, >>>>>> Below are a few comments after a quick look at your webrev: >>>>>> >>>>>> 1. usr/src/pkgdefs/Makefile >>>>>> Please add the entry in alphabatical order. >>>>>> >>>>>> 2. METADATA >>>>>> Please check if the license needs to be mentioned as GPL v2 >>>>>> >>>>>> 3. Makefile.sfw Hard-coding of version can be avoided and this >>>>>> info can be picked up from METADATA, something like: >>>>>> VER = $(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) >>>>>> >>>>>> 4. install-convmv >>>>>> a) Usually the install scripts are named as install-sfw. >>>>>> b) It is recommended to use /usr/bin/ksh3 instead of /bin/sh. >>>>>> This allows you to add: >>>>>> # stop at first error >>>>>> set -o errexit >>>>>> And to call "source" instead of "." in this line: >>>>>> source ${SRC}/tools/install.subr >>>>>> c) Please change the copyright year to 2009. >>>>>> d) Since there is only one binary and one man page that is being >>>>>> installed. In my opinion, the 'for' loops are not required e) >>>>>> The binary should be installed as executable 'E' and not as normal >>>>>> files as below: >>>>>> _install E ${i} ${BINDIR}/${i} 555 >>>>>> 'N' is generally used for normal files. >>>>>> >>>>>> 5. SUNWconvmv/Makefile >>>>>> Since you are not using the default depend file, please remove >>>>>> the empty DATAFILES entry. >>>>>> >>>>>> 6. prototype_com >>>>>> Entries for usr/share and usr/share/man dirs also need to be added. >>>>>> >>>>>> Regards >>>>>> Muktha >>>>>> >>>>>> Suresh Chandrasekharan wrote: >>>>>> >>>>>> >>>>>>> Changes at >>>>>>> http://cr.opensolaris.org/~suresh/convmv_webrev/ >>>>>>> >>>>>>> Suresh Chandrasekharan wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> Hi All, >>>>>>>> Pl. review the changes to integrate convmv 1.14 >>>>>>>> Regards, >>>>>>>> Suresh >>>>>>>> >>>>>>>> >>>> >>>> >>> _______________________________________________ >>> sfwnv-discuss mailing list >>> sfwnv-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss >>> >>> >> >> _______________________________________________ >> sfwnv-discuss mailing list >> sfwnv-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss >> > > ------------------------------------------------------------------------ > > _______________________________________________ > sfwnv-discuss mailing list > sfwnv-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss >
