See below ... paul
Vivek Titarmare wrote: > > Also I have uploaded the updated webrev in the same link. > (http://cr.opensolaris.org/~vivekrt/6833852-pywbem ) > From: Paul Cunningham [mailto:paul.cunningham at tadpole.com] > 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/ > 1. usr/src/lib/pywbem/METADATA > 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. I still think you should put in a real url on the SRC: line 2. usr/src/Targetdirs & usr/src/pkgdefs/Makefile Needs resyncing with the gate by the looks of it. > 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/"; The following are still wrong ... pywbem/Makefile.sfw pywbem/install-sfw and various others (if not all) > 4. usr/src/lib/pywbem/install-sfw Line ... 65 install_dir "*" ${ROOTPYWBEM} 755 444 is this correct ? > Should ... > /usr/lib/pywbem and /usr/share/doc/pywbem be in Targetdirs? > > [VIVEK] yes doc was missing in the Targetdirs. Now it is updated. I think /usr/lib/pywbem should be in Targetdirs as well! > 6. usr/src/lib/pywbem/sunman/pywbem.3 I'm not sure you say the following in a sun man page ... 41 Pywbem also provides a Python provider interface, and is the fastest and easiest way to write providers on the planet. ie. the bit about ... "fastest and easiest way to write providers on the planet" Do you need to refer to the version in a man page ? ... 42 The latest version of Pywbem is 0.7 ...... > 7. usr/src/pkgdefs/SUNWpywbem/pkginfo.tmpl > This doesn't look like a template file, it has ARCH="i386" > defined ? this still doesn't look right > Add version number yo end of DESC= line > > [VIVEK] version added It's not in the webrev! > 8. usr/src/pkgdefs/SUNWpywbem/copyright > Is this the correct Sun disclaimer statement for LGPL ? > > [VIVEK] LGPL Disclaimer Notice is prepend in the copyright file. Does it need both of those two disclaimer statements? > 9. usr/src/pkgdefs/SUNWpywbem/depend > Does this need to include the normal default dependencies > also ? > > [VIVEK] added default dependencies Does it need the zip dependencies ? 10. usr/src/lib/pywbem/Makefile.sfw What is this line doing for you ? ... 45 (cd $(VER); $(PYTHON) setup.py install --install-lib .. $(SRC)/lib/pywbem/$(VER)) it looks to me as though you are installing stuff back into your build dir! -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
