Note: changed 'Subject' line Here are some comments to start with ...
Paul Brian Gupta wrote: ... cut ... > > I guess that mean I need a code review. Here is the upload: > http://cr.opensolaris.org/~brandorr/firstreview/webrev/ > You have no packaging stuff in usr/src/pkgdefs, eg. ... usr/src/pkgdefs/Makefile usr/src/pkgdefs/SUNWgvim?/* usr/src/cmd/gvim/sunman-stability - will SUNW package name be SUNWvim or SUNWgvim? You need to add gvim into .. usr/src/cmd/Makefile You need to add usr/src/cmd/gvim/METADATA usr/src/cmd/gvim/Makefile.sfw Explicitly define 'configure --prefix=..'. Use the predefined value for '--prefix=..' from Makefile.master; or even better still use the stuff from CONFIGURE_OPTIONS, eg ... ./configure $(CONFIGURE_OPTIONS) see other recent integrations for examples, eg cups. You might want to apply 'Roland Mainz' comments ... - Use "env - ..." and not "env ..." in the Makefiles to make sure "configure" & "make" only see the env variables they should really get(and not pick-up any random env variable) - Use either $(SHELL) or /usr/bin/bash for "configure" calls (so we know which one is used and "configure" doesn't pick one itself). You could extract the version info from the METADATA file as per 'Christopher Mi' comment .. - Use the method define in Makefile.master since you have a standard METADATA file, eg. .. VER= $(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) TARBALL= $(VER).tar.bz2 usr/src/cmd/gvim/install.sfw You might want to apply 'Roland Mainz' comments ... - use /usr/bin/ksh93 or /usr/bin/bash for install-* and add a $ set -o errexit # at the beginning and replace ".${SRC}/tools/install.subr" with "source ${SRC}/tools/install.subr" (the idea is to catch failures in the script and abort it at that point, right now the script will just continue). As an example for all stuff required look at say ... http://cr.opensolaris.org/~jwalker/meld/ -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Computer Products
