Srirama, See comments below from my quick skip through ...
Paul Srirama Sharma wrote: > > Requesting a code review for SimpleWBEM. SimpleWBEM provides command > line tools for building CIM (Common Information Model) providers that > are compatible with several CIM server implementations like OpenPegasus. > > Webrev: http://cr.opensolaris.org/~srirama/simplewbem/ > Bug: http://bugs.opensolaris.org/view_bug.do?bug_id=6772743 > > Please note that this is targeted to get integrated in to snv_105 build. === Start of Comments ==== 1. usr/src/cmd/simplewbem/METADATA You might want to add the 'NAME:' field see ... http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines 2. usr/src/cmd/simplewbem/*.1 Did you write these man pages, if so you might need to add the Sun copyright and CDDL stuff. 3. usr/src/cmd/simplewbem/Makefile.sfw You could extract the VER= stuff from the METADATA info, see recent sfwnv package integrations. Change 'env ' to 'env - ' throughout It's not normal to have specific $(VER)_*/config.status machine arch lines - so why have you got them? Do you need separate 'clean32:' & 'clean64:' rules? You could just have ... clean: -rm -rf $(VER) $(VER64) 4. various files - #pragma ident ... You don't need the 'pragma', I thing that's normally only required for c, etc, code files - see previous Jim Walker comments. 5. usr/src/cmd/simplewbem/install-sfw & usr/src/cmd/simplewbem/install-sfw-64 You could pass in the VERS= stuff as an environment variable from the Makefile.sfw Roland Mainz wrote: > 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) Do you need the 'write permission' set on the library files ? if not remove it. 6. usr/src/pkgdefs/SUNWsimplewbem/Makefile Remove the null DATAFILES= line 7. usr/src/pkgdefs/SUNWsimplewbem/depend Move the Copyright lines down to after the 'CDDL HEADER END' header. And correct copyright year. 8. usr/src/pkgdefs/SUNWsimplewbem/pkginfo.tmpl On the 'DESC= line move the version to the end of the line, eg. DESC=" .......... (1.2.4)" 9. usr/src/pkgdefs/SUNWsimplewbem/prototype_com & usr/src/pkgdefs/SUNWsimplewbem/prototype_i386 & usr/src/pkgdefs/SUNWsimplewbem/prototype_sparc Do you need the 'write permission' set on the library files ? if not remove it. === End of Comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
