Vivek, See comments below ....
Paul Vivek Titarmare wrote: > I have posted a webrev for package "pywbem" which I am porting to > OpenSolaris and would like to request a code review. Please see below link > http://cr.opensolaris.org/~vivekrt/6833852-pywbem/ === Start of Comments === 1. usr/src/lib/pywbem/METADATA & usr/src/lib/pywbem/pywbem-0.7.tar.gz Line ... 6 SRC: pywbem-0.7.0.tar.gz from http://pywbem.sf.net/ says version 0.7.0 (and 4 VERSION:) but your tarball is 0.7. Why ? SRC: line, why no specify the direct link ? ... "http://kent.dl.sourceforge.net/sourceforge/pywbem/pywbem-0.7.0.tar.gz"; this uncompresses into pywbem-0.7.0. 2. usr/src/lib/pywbem/Makefile.sfw Lines .. 28 #VER= $(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) 29 VER=pywbem-0.7 why have you hardcoded this ? when the tarball uncompresses to dir pywbem-0.7.0 Define 'TARBALL=$(VER).tar.gz' and then use $(TARBALL) instead of '$(VER).tar.gz' throughout 3. Top-of-files (various) Cosmetic: Change top-of-file so it conforms to (missing line-space, etc) ... "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; 4. usr/src/lib/pywbem/install-sfw Line 1, change to /usr/bin/ksh93 Line .. 102 cd ${BASEDIR} is this putting you in the correct place for the following stuff ? Line .. 103 install_dir "pywbem.txt" ${ROOTPYWBEM_DOC} 755 444 is "pywbem.txt" correct for a directory ? Should ... /usr/lib/pywbem and /usr/share/doc/pywbem be in Targetdirs? 5. usr/src/lib/pywbem/cim_schema_2.20.0Experimental-MOFs.zip Where is this used? 6. usr/src/lib/pywbem/sunman/pywbem.3 Line ... 68 Availability SUNWpyWBEM elsewhere its SUNWpywbem 7. usr/src/pkgdefs/SUNWpywbem/pkginfo.tmpl This doesn't look like a template file, it has ARCH="i386" defined ? Add version number yo end of DESC= line 8. usr/src/pkgdefs/SUNWpywbem/pkginfo.tmpl Is this the correct Sun disclaimer statement for LGPL ? 9. usr/src/pkgdefs/SUNWpywbem/depend Does this need to include the normal default dependencies also ? 10. usr/src/pkgdefs/SUNWpywbem/prototype_i386 & usr/src/pkgdefs/SUNWpywbem/prototype_sparc Cosmetic: add package name in comment for consistency as in .. "http://cr.opensolaris.org/~vivekrt/6816355-xstream/usr/src/pkgdefs/SUNWxstream/prototype_sparc.html"; === End of Comments ===== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
