Jim Walker wrote: > Huie-Ying Lee wrote: >> >> I have updated the code to address all the comments that I have >> received for the pyOpenSSL package. >> The updated webrev is located at the same place, >> http://cr.opensolaris.org/~hylee/pyopenssl. Please take a look and >> let me know if you have any comments. > > Here's a few comments... > Hello Jim,
> Please update all the files with the correct CDDL header and > copyright header formats. Here's an example: > http://cr.opensolaris.org/~jwalker/meld/usr/src/cmd/Makefile.html > I checked all the files, but I couldn't find a file that does not have a correct CDDL header and copyright header formats. Could you please tell me which file is incorrect ? > usr/src/lib/Makefile > 85 pyopenssl > - unless there are build ordering issues, normally > packages are in alphabetical order, after here may > be better: OK. Fixed. > 80 pam_pkcs11 \ > - also update the CDDL and copyright header to correct > format. > As I mentioned above, the CDDL and copyright headers of usr/src/lib/Makefile are correct. > usr/src/lib/pyopenssl/METADATA > - a brief description should be used for the name, > maybe something like: > pyOpenSSL - Python interface to the OpenSSL library > OK. Fixed. > usr/src/lib/pyopenssl/Makefile.sfw > - I'm going to think more about this Makefile, > but it may be better to use a install-sfw file > to do the file transfer work > > You need to add a man page. > The reasons that a man page was not added are listed below: 1. All the vendor packages for Python2.4 are located in the standard /usr/lib/python2.4/vendor-packages directory, so it should be of any problems for a Python programmer to find this package and its corresonding document in /usr/lib/python2.4/vendor-packages/OpenSSL/doc. 2. The deliverables of this package has been approved by PSARC, without the requirement for a man page. 3. There are no man pages supplied by any existing Python vendor packages that I could find. > usr/src/pkgdefs/SUNWpython-pyopenssl/copyright > - I assume you need a disclaimer and copyright > section like is included here: > http://cr.opensolaris.org/~jwalker/meld/usr/src/pkgdefs/SUNWmeld/copyright.html > > > The copyright file in the usr/src/pkgdefs/SUNWpython-pyopenssl directory is copied directly from the original pyOpenSSL tarball. I don't think I should alter it. > usr/src/pkgdefs/SUNWpython-pyopenssl/prototype_com Did you mean "usr/src/pkgdefs/SUNWpython-pyopenssl/pkginfo.tmpl" ? > 33 NAME="Python language bindings for OpenSSL" > 41 DESC="Python language bindings for OpenSSL (0.8)" > - is it really just language bindings, or is this > name/desc more accurate? > "Python interface to the OpenSSL library" OK. Fixed. > usr/src/pkgdefs/SUNWpython-pyopenssl/prototype_i386 > - you should use the correct file format like this: > http://cr.opensolaris.org/~jwalker/meld/usr/src/pkgdefs/SUNWmeld/prototype_i386.html > > > The file format in this file is correct. The difference between this file and your example are only comments. However, I decided to add those comments back to this file, so that it will be more informative. > usr/src/pkgdefs/SUNWpython-pyopenssl/prototype_sparc > - you should use the correct file format like this: > http://cr.opensolaris.org/~jwalker/meld/usr/src/pkgdefs/SUNWmeld/prototype_sparc.html > > > Same as above, I added the comments back to this file also. > usr/src/pkgdefs/SUNWpython-pyopenssl/prototype_com > - I think this section should be in prototype_i386 > 48 d none usr/lib/python2.4/vendor-packages/OpenSSL/64 0755 root bin > 49 f none usr/lib/python2.4/vendor-packages/OpenSSL/64/SSL.so 0555 > root bin > 50 f none usr/lib/python2.4/vendor-packages/OpenSSL/64/crypto.so > 0555 root bin > 51 f none usr/lib/python2.4/vendor-packages/OpenSSL/64/rand.so 0555 > root bin > No, these are for SPARC (sparcv9) also. Please see /usr/lib/python2.4/vendor-packages/64 on a SPARC machine. > Cheers, > Jim Thank you for the comments. I have updated the code and the updated webrev is located at http://cr.opensolaris.org/~hylee/pyopenssl. Huie-Ying
