See below for comments ... Paul
Martina Tomisova wrote: > > finally there is the newest code version and I hope it won't change anymore. > Please have a look at it and tell me if you see something bad :) > > http://cr.opensolaris.org/~martina/ngrep/ > === Start of Comments ==== 1. usr/src/cmd/ngrep/METADATA You might want to make its contents comply to the METADATA guidelines in .. "http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines"; 2. usr/src/cmd/ngrep/Makefile.sfw You could extract the pkg name/version from the METADATA, using something like ... VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) TARBALL =$(VER).tar.bz2 The DESTDIR= is not used in install-sfw so remove it .. 45 (DESTDIR=$(ROOT) $(SHELL) -e './install.sfw') Remove the 'pragma' on the SCCS ident line (25) - and in other files. 3. usr/src/cmd/ngrep/install.sfw You could pass the VERS= value in from the Makefile.sfw as an environment variable, eg something like .. in Makefile.sfw 45 (VERS=$(VER) $(SHELL) -e './install.sfw') in install-sfw 29 VERS=${VERS} 4. usr/src/pkgdefs/SUNWngrepr/depend You can remove this file, because ... a) it looks like the default 'depend' file b) you have DATAFILES= depend in SUNWngrepr/Makefile so it will use the default 'depend' file. 5. usr/src/pkgdefs/SUNWngrepr/r.rbac Is this the default file ? 6. usr/src/pkgdefs/SUNWngrepu/Makefile You have your own 'SUNWngrepu/depend' file so remove the line .. 29 DATAFILES= depend 7. usr/src/pkgdefs/SUNWngrepu/depend Should this depend on SUNWngrepr? Have you checked you have no other dependencies with the dependency checker script? This currently looks like the default 'depend' file. Move 'Copyright' lines to after the "CDDL HEADER END" header and correct year. 8. usr/src/pkgdefs/SUNWngrepu/prototype_com Do you need the write permission set on the usr/sbin/ngrep file? 9. ngrep-1.45.tar.bz2 is not in your webrev (though it is in your 'putback -n' list, so is probably okay) === End of Comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
