Hi Daria, I have incorporated the review comments. Please see responses inline.
Daria Mehra wrote: > > 1) Your METADATA file needs to follow standard outlined on this wiki: > http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines > Please use CAPS for field names, add PROGRAM field, change "Package > name" to PACKAGE, add SRC url, change "GPL" to "GPL v2", and change > value of BUGTRAQ from your CR ID to the product/category/subcategory > people should use when filing bugs against konkret. > Done. > 2) Per Jim Walker, do not use "pragma ident" keywords in files other > than source files. In Makefiles, install scripts and such, use "ident" > (without "pragma"). > Done. > 3) In Makefile.sfw, you can avoid hard-coding program name and version > and instead extract this info from METADATA, something like: > VER = $(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) > For the same effect in install-sfw script, you'd need to pass VERS as > an environment variable from the Makefile. See example in webalizer > Makefile.sfw and install-sfw. > Done. > 4) Why do you overwrite SFW_PATH setting in Makefile.sfw? If it's > necessary, please add a comment to explain what tools your build is > getting off the special path. > Have modified not to overwrite the SFW_PATH in Makefile.sfw. > 5) In Makefile.sfw, it is advisable to use $(SHELL) instead of $(SH) > to ensure predictability of which shell will get called. You may want > to call configure as "$(SHELL) ./configure" for the same reason. > Done. > 6) In install-sfw and install-sfw-64, it is recommended to use > /usr/bin/ksh93 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 > The purpose of this is to make the install script exit on error > instead of silently forging ahead. See examples in webalizer and a few > other packages using ksh93. > Done. > 7) I don't believe you need sunman-stability stuff because you are > creating a new manpage, rather than using one from open-source > distribution. From the package guidelines wiki: > > http://ostest.central.sun.com/wiki/index.php/Package_Delivery_Project#3._Develop_Software > > > If you do create a new man page, the sunman-stability file is not > needed. > Have removed sunman-stability and have added CDDL and Sun copyright headers to the new manpages. > 8) Fix copyright year in the CDDL headers, it should be 2008. CDDL > headers should be consistent across all your files. The easiest way is > to copy the current header from here: > /net/onnv.sfbay/export/onnv-tw-gate/usr/src/prototypes/prototype* > Done. > 9) You are using the default 'depend' file in the package, but your > build appears to pull in some headers from Pegasus. Is this only a > compile-time dependency? Please verify that your binary does not have > any runtime dependencies (beyond the default ones such as libc, etc). > You can run "ldd" to see what it's linking against. If runtime > dependency on another package exists, please create your own depend > file (copy the default and add packages you explicitly depend on). > Then remove this line from your package Makefile: > DATAFILES=depend > Yes, konkret has only compile-time dependency on Pegasus. Hence was using the default 'depend' file in the Makefile. Also, in usr/src/cmd/Makefile, 'konkretcmpi' entry has been put after 'cimserver' (Pegasus) entry. Please let know if an explicit entry 'konkretcmpi: cimserver' is required at the end of the Makefile. Please review the changes. webrev: http://cr.opensolaris.org/~muktha/konkretcmpi/ Regards Muktha
