Paul Cunningham wrote:
> The ONNV bit looks okay to me > Great. > > === Start of Comments ==== > > 1. usr/src/cmd/ntpd/METADATA > This needs updating to conform to the heads-up of last week, > as in ... > "http://wikis.sun.com/display/SFWNotes/METADATA"; > > Line ... > 8 SUPPORT: Full > is that the correct support classification? Yes, that is the correct class. > > > 2. tarball ntp-dev-4.2.5p161.tar.gz > This is not in the webrev? No, should it be? It is the as downloaded. > > 3. version ntp-dev-4.2.5p161 > The source pkg's website says there is version > 'R.C.: 4.2.4p7-RC7 2009/05/12', so should you be using that > now? No. I am shooting for 4.2.5, not 4.2.4. > > 4. usr/src/cmd/ntpd/install-sfw > Some of the directories you are '${INSTALL} -d'ing, are > already in the 'Targetdirs' file so you don't need to > install then here. > > And the other directories created here could probably also > be put in 'Targetdirs'. > > You could have used the '_install' macro rather than '${INSTALL}' > for installing files. Didn't know that. I'll change it. What makes something appropriate or inappropriate for Targetdirs? > > Change as ... > Roland Mainz wrote: > > use /usr/bin/ksh93 for install-sfw* and 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) Done. > > 5. usr/src/cmd/ntpd/Makefile.sfw > & usr/src/cmd/ntpd/install-sfw > Line ... > 69 (cd $(VER); env - $(CCSMAKE) all $(TARGET_ENV)) && \ > install-sfw $(VER) > is the 'make install' installing different stuff to > 'install-sfw' ? No. "make install" just does a "make all" and install-sfw. There is no "make install" done inside the untarred directory. > > 6. Tops of files (various files) > Cosmetic: Change so that they conform to those in ... > "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; > as per > "http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines"; I am not sure what you are saying. The only files that don't have CDDL and Copyright info are the Patch files. Looking at the other projects in SFW, none of them have CDDL or copyright in the patch files. Or am I misunderstanding you? > > 7. patches > I haven't looked at these - I assume they are okay > > 8. usr/src/cmd/ntpd/Solaris/ntp > Why '/sbin/sh' rather than '/usr/sbin/sh' ? Seems to the overwhelmingly predominant choice for SMF start-up methods. I just followed the existing methods. > > 9. man pages > Note, I haven't read their content > > 10. usr/src/cmd/ntpd/Solaris/ntp-keygen.1m > & usr/src/cmd/ntpd/Solaris/ntpq.1m > & usr/src/cmd/ntpd/Solaris/ntptime.1m > & usr/src/cmd/ntpd/Solaris/ntptrace.1m > Does this need the 'Interface Stability' stuff added? They are already all there. I don't use a script to add them. > > And the where to find source added ? ... > "Source for xxx is available on http://opensolaris.org Added to the METADATA file. > > 11. usr/src/cmd/ntpd/Solaris/ntp.conf.4 > & usr/src/cmd/ntpd/Solaris/ntprc.4 > Does this need the sunman stability stuff added? Done. > > 12. usr/src/cmd/ntpd/Solaris/ntp.xml > The Copyright lines should be after the > 'CDDL HEADER END' header Done > > 13. usr/src/pkgdefs/SUNWntpr/Makefile > You have your own pkg copyright files so you > don't need the 'copyright' in DATAFILES= > Done. > 14. usr/src/pkgdefs/SUNWntpr/depend > This looks like the default 'depend' file so > you can delete it as SUNWntpr/Makefile says use the > default one. Done. > > 15. usr/src/pkgdefs/SUNWntpr/pkginfo.tmpl > & usr/src/pkgdefs/SUNWntpu/pkginfo.tmpl > Add source pkg version at the end of the DESC= line, eg ... > DESC=".................. (x.x.x)" Done. > > 16. usr/src/pkgdefs/SUNWntpu/copyright > & usr/src/pkgdefs/SUNWntpr/copyright > Does this need to include the lines 19 to 55 ? > > Should it also include a copy of the 'Modified BSD' > licence (or are lines 5-15 that) Modified BSD is not longer accepted. It is now BSD-LIKE. And, yes, 5-15 is the license. > > 17. usr/src/pkgdefs/SUNWntpu/depend > Does this really have no other dependencies. What about > SUNWntpr and others. If not it should use the default 'depend' Added correct dependencies. > > 18. usr/src/cmd/ntpd/Solaris/ntprc.4 > Have you delivered this in a SUNW pkg, I can't see where? It is a file format of an optional file, not delivered. > > === End of Comments ====== -- blu "Mark my words, nanotechnology is going to be huge!" ---------------------------------------------------------------------- Brian Utterback - Solaris RPE, Sun Microsystems, Inc. Ph:877-259-7345, Em:brian.utterback-at-ess-you-enn-dot-kom
