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 =====

Reply via email to