Resent because I'm not sure it got sent out (Mark, did you get it?) ..
On Tue, 2008-06-10 at 15:47, Paul Cunningham wrote:
> Mark,
>
> See below for my comments from my quick skip through ...
>
> Paul
>
> On Mon, 2008-06-09 at 22:03, Mark Fenwick wrote:
> > This is a request for code review for:
> >
> > 6712365 Intergration of stunnel into Solaris
> >
> > http://cr.opensolaris.org/~markfen/stunnel/webrev/
> >
> > I still have some work to do, namely:
> >
> > Update software tarball to 4.24
> > Move man page to section 1.
> > Write a PSARC case.
> >
> > All the same, I'd still appreciate feedback on whats here now.
> >
> > The man page that comes with the tarball is a bit vague in places and lacks
> > examples, it does not cover the use of stunnel with SMF, which is unique to
> > Solaris. Its also optional.
> >
> > I don't want to edit the existing man page as that makes updating the
> > tarball hard work as these edits would need to be reapplied each time.
> > Whats the best approach for enhancing the man page ?
> >
> > Write a new one and reference the original version via a URL ?
> > Have two man pages ?
> > Something else ?
>
> === Start of Comments ===
>
> 1. usr/src/cmd/stunnel/Makefile.sfw
> Could you use the '--prefix=..' value predefined in
> Makefile.master? see example in ..
> http://cr.opensolaris.org/~rayx/erlang/webrev/usr/src/cmd/erlang/Makefile.sfw.html
>
> 2. usr/src/cmd/stunnel/stunnel.xml
> Shouldn't the copyright lines come after the
> 'CDDL HEADER END' header.
>
> 3. usr/src/pkgdefs/Makefile
> Looks as though it need resyncing with gate otherwise
> if looks as though you are deleting stuff.
>
> 4. usr/src/pkgdefs/SUNWstunnelr/depend
> This looks like the default 'depend', I don't
> think you meant to check it in!
>
> 5. usr/src/pkgdefs/SUNWstunnelu/prototype_i386
> & usr/src/pkgdefs/SUNWstunnelu/prototype_sparc
> & usr/src/pkgdefs/SUNWstunnelr/prototype_i386
> & usr/src/pkgdefs/SUNWstunnelr/prototype_sparc
> Copyright lines are wrong and it should probably
> come after the 'CDDL HEADER END' header.
>
> 6. usr/src/pkgdefs/SUNWstunnelu/Makefile
> You don't need the 'DATAFILES= depend' as it has
> its own 'depend' file.
>
> Copyright line is of the wrong format.
>
> 7. usr/src/pkgdefs/SUNWstunnelu/depend
> Shouldn't this depend on SUNWstunnelr?
>
> Copyright line are wrong and it should probably
> come after the 'CDDL HEADER END' header.
>
> 8. usr/src/pkgdefs/SUNWstunnelu/pkginfo.tmpl
> Copyright year is wrong
>
> 9. man page
> You need to sort the sunman-stability stuff
> appropriately
>
> === End of Comments =====
