Dean,

Here are my comments from my quick skip through, it mainly looks good 
though; see below ...

Paul


Dean Roehrich wrote:
> I've submitted a webrev to http://cr.opensolaris.org/~roehrich/quilt/
> to integrate quilt into SFW.
> 
> Quilt is a tool to manage a stack of patches; it's the predecessor to
> Mercurial Queues (mq).

=== Start of Comments ====

1. usr/src/cmd/Makefile
     & usr/src/pkgdefs/Makefile
    Why is the fping stuff commented out??

2. usr/src/cmd/quilt/install-sfw
    cosmetic: add line space after line 1, ie.
      1 #! /usr/bin/ksh93
        #
      2 # CDDL HEADER START
      3 #

3. usr/src/pkgdefs/SUNWquiltr/depend
    You can remove this as you have told the build to use the
    default 'depend' in SUNWquiltr/Makefile

4. usr/src/pkgdefs/SUNWquiltr/pkginfo.tmpl
     & usr/src/pkgdefs/SUNWquiltu/pkginfo.tmpl
    Remove the version from the NAME= line and put it on
    the DESC= line, eg. ...
       NAME="Quilt"
       DESC="Quilt - tool to manage series of patches (0.47)"

5. usr/src/pkgdefs/SUNWquiltu/Makefile
    Remove the 'DATAFILES = depend' line unless you really
    have no other dependencies, if not remove SUNWquiltu/depend
    instead (see (6)).

6. usr/src/pkgdefs/SUNWquiltu/depend
    Shouldn't this depend on SUNWquiltr?
    Have you checked it has no other dependencies with the
    dependency checker script?
    Move the 'Copyright' lines to after the 'CDDL HEADER END'
    header. And change year.

7. usr/src/cmd/quilt/Makefile.sfw
    Do you really need line ? ...
     59   @find . -name core -exec rm -f {} \;
    I think you can also extract the version from the METADATA
    file ...
     27 APP = $(COMPONENT_NAME:sh)-0.47
    Delete extra line space ..
     88

=== End of Comments ======
-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit

Reply via email to