Hi Paul, See comments inline
Paul Cunningham wrote: > Amanda, > > See comments below ... > > Paul > > Amanda Waite wrote: >> [resending to include sfwnv-discuss and setting reply-to to >> webstack-discuss] >> >> Hi all, >> >> Please review the webrev for the Lighttpd integration (CR6687382) at >> http://cr.opensolaris.org/~tekgrrl/lighttpd14 > > ====== Start of Comments ========================= > > 1. usr/src/Targetdirs + > usr/src/cmd/Makefile > You seem to have removed the 'nethack' stuff ! This is strange, the nethack stuff is in my workspace at the moment. The diffs definitely show that it was removed though. I'm stuck on b82 at the moment so I'm going to get the system upgraded. I'll look out for things like this in future. > > 2. usr/src/cmd/lighttpd14/Makefile.sfw > Why is the compile bit under 'install-lighttpd:' and not > under 'all:'? > I don't think you would normally expect 'make all' to also do > the install? I copied another Makefile, I'm changing it so that the compile is under "all" and there's a separate "install" target > > 3. usr/src/cmd/lighttpd14/Solaris/fcgi-php.conf > If this is specific to Solaris, should it have Sun Copyright > stuff in it? > > 4. usr/src/cmd/lighttpd14/Solaris/http-lighttpd14.xml > Sun Copyright message year/format wrong > > 5. usr/src/cmd/lighttpd14/Solaris/index.html > If this is specific to Solaris, should it have Sun Copyright > stuff in it? > > 6. usr/src/cmd/lighttpd14/Solaris/lighttpd.1m.sunman > I don't think the format of the Sun copyright message is > correct. > > 7. usr/src/cmd/lighttpd14/install-sfw + > usr/src/cmd/lighttpd14/lighttpd.build.env + > usr/src/pkgdefs/SUNWlighttpd14r/Makefile + > usr/src/pkgdefs/SUNWlighttpd14r/pkginfo.tmpl + > usr/src/pkgdefs/SUNWlighttpd14r/prototype_i386 + > various others > Copyright year is wrong I'll address all of the copyright issues. > > 8. usr/src/pkgdefs/SUNWlighttpd14r/pkginfo.tmpl + > usr/src/pkgdefs/SUNWlighttpd14u/pkginfo.tmpl > I quiry whether the 'NAME=' and 'DESC=' lines should > have the packages version on them ???? The packages are SUNWlighttpd14[r|u] I feel that the versioning of the package means that the name and description should also include the version. This is the same as has been done with other versioned packages. > > 9. file 'ident' strings > Are the sccs 'ident' strings correct (check all files) as some > of them are no the first version, eg. in > usr/src/pkgdefs/SUNWlighttpd14r/prototype_com, etc. This is lack of experience telling, I had no idea what to do with the ident strings, in fact I passed them over not realising their importance. I shall address this on all of the files. > > 10. usr/src/pkgdefs/SUNWlighttpd14r/prototype_i386 + > usr/src/pkgdefs/SUNWlighttpd14r/prototype_sparc > package name wrong SUNWvim ??? I missed this, corrected now. Thanks for your feedback, I'll try not to make these basic errors in future. Amanda > > ====== End of Comments =========================== > -- Amanda Waite ISV-Engineering OSS, Sun Microsystems Tel: +44 (0)1252 420693 Mobile: +44 (0)7802 175732
