Dan, See below for some comments ...
Paul Dan Hain wrote: > Hi all, > > Please review the code for fping's integration into the sfw consolidation > > The webrev is located at: http://cr.opensolaris.org/~dhain/fping/ ==== Start of Comments ==== 1. usr/src/cmd/fping/Makefile.sfw Could you use the '--prefix=..' value predefined in Makefile.master? see example in .. http://cr.opensolaris.org/~rayx/erlang/webrev/usr/src/cmd/erlang/Makefile.sfw.html Also is there a predefined MAKE, if so could you use that? Do you need the 'real-all:' rule, couldn't it just be 'all: all32 all64' instead? Do you need to do the 'cp ../fping.1m fping.1m' in '$(VER64)/configure:' as you only install in from $(VER) ? 2. METADATA You don't seem to have a METADATA file. 3. usr/src/cmd/fping/install-sfw Do you need to define LOCALEDIR= & INFODIR=, I don't think they are used? Why use 'i=...' when its only used once? You don't seem to have a sunman-stability file (in the webrev). Copyright year is wrong. 4. usr/src/cmd/fping/install-sfw-64 Copyright year is wrong. What happens on a sparc build - amd64/fping ? 5. usr/src/common/rbac/exec_attr Copyright year needs changing. 6. usr/src/pkgdefs/Makefile You haven't added 'SUNWfpingr' 7. usr/src/pkgdefs/SUNWfping/Makefile & usr/src/pkgdefs/SUNWfping/pkginfo.tmpl & usr/src/pkgdefs/SUNWfping/prototype_sparc Copyright year is wrong. The sccs ident stuff looks wrong. 8. usr/src/pkgdefs/SUNWfping/copyright Should this include some copyright years? Why is it not the same as SUNWfpingr/copyright? 9. usr/src/pkgdefs/SUNWfping/prototype_sparc Some there be sparc 64 bit stuff? 10. usr/src/pkgdefs/SUNWfpingr/prototype_com SUNWmkcdr - name comment wrong. ==== End of Comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Computer Products
