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 > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/sfwnv-discuss/attachments/20090205/82b836e0/attachment.html>
