see below... Paul Cunningham wrote: > See below ... > > Paul > > Brian Utterback wrote: > >> http://cr.opensolaris.org/~blu/ntpv4/sfwnv-webrev/ >
>>> >>> 2. tarball ntp-dev-4.2.5p161.tar.gz >>> This is not in the webrev? >> >> No, should it be? It is the as downloaded. > > yes - otherwise how does it get into the gate on putback It is in the gate, just not in the webrev. It is in webrev.NOT. >>> 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. > > whoops I read that wrong (read 'all' as 'install'). > But having said 'whoops' shouldn't therefore .... > the 'install-target:' rules depend on the 'all:' rule then you > wouldn't need the '$(CCSMAKE) all $(TARGET_ENV)) &&' in .. > 69 (cd $(VER); env - $(CCSMAKE) all $(TARGET_ENV)) && \ > install-sfw $(VER) Good point. Done. > >>> 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? > > I'm talking about the layout in the tops of your files - they are > cosmetically different to the prototypes; and yes lots of those already > in the sfw gate are wrong. But why propagate that wrongness. Do you mean the '\" t at the top of the man pages? Other than that, I don't see any difference between the new files I introduced and the prototypes. I have fixed the man pages though. >>> 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. > > either I'm going blind or ..... > > 209 ATTRIBUTE TYPE^GATTRIBUTE VALUE > 210 _ > 211 Availability^GSUNWntp4u > 212 .TE > 213 .PP > > where's 'Interface Stability' ? Interesting. Webrev apparently doesn't like something, probably the embedded ^G. On some systems the webrev ends up omitting a couple of lines. The man pages actually all have the interface stability. The new webrev should have them all. > > >>> And the where to find source added ? ... >>> "Source for xxx is available on http://opensolaris.org >> >> Added to the METADATA file. > > its needed, I believe in the bottom of the man page also. Perhaps this is the same issue. It is already in all of the man pages. I misread what you wrote before. The METADATA file has the link to ww.ntp.org for the source. The man pages all reference opensolaris.org. Actually, though, the license for NTP doesn't require this. > >> >>> >>> 16. usr/src/pkgdefs/SUNWntpu/copyright >>> & usr/src/pkgdefs/SUNWntpr/copyright >>> Does this need to include the lines 19 to 55 ? > > ? Good point. Those lines are not really part of the copyright, although the distro has them in the copyright file. They are kind of annoying during installation. I have removed the extra lines. > >>> 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. > > have you changed it in the METADATA then to "BSD-LIKE" then? Yes. > > >>> 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. > > so if its not used why not remove it - to stop confusion The file is used, it just isn't delivered. Somewhat similar to the ntp.conf file, although two example files are delivered for ntp.conf. > > > have you got an updated webrev ? It should be there now.q -- 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
