Vivek,

See comments below ...

Paul

Vivek Titarmare wrote:
> I have posted a webrev for package "pdsh" which I am porting to Nevada 
> and would like to request a code review. Please see the link below 
> 
> http://cr.opensolaris.org/~vivekrt/6833847-pdsh/

1. usr/src/cmd/pdsh/METADATA
    Line ...
      4 LICENSE:          GPL, LGPL, BSD
    but on the pkg's home page (before it disappears of the screen)
    it justs says GPL ?

2. usr/src/cmd/pdsh/Makefile.sfw
    Why are you applying patches after it has been configured, couldn't
    they me done first?

    The 'configure' is normally done in a rule of its own rather
    than with tarball unpack.

    Set $(SHELL) before running 'configure', eg ...
       $(SHELL) ./configure $(CONFIGURE_OPTIONS)

    You could use .patched target automating patching and TARBALL
    unpacking. See cups or wireshark.

    As you have used 'make install' do you need to run
    'protofix' to fix installed file permissions?

3. usr/src/cmd/pdsh/sunman/dshbak.1
      & usr/src/cmd/pdsh/sunman/pdcp.1
    Add at end where they can find the source on opensolaris

4. Man pages
    I haven't checked these as these are too hard to read in
    this format

5. usr/src/pkgdefs/SUNWpdsh/copyright
    As per comment 1, do you need all this stuff?

6. depend
    Have you checked there are no other dependencies other than the
    defaults?

7. usr/src/pkgdefs/SUNWpdsh/prototype_com
    I don't think you should be delivering the *.la and *.a
    files ?

    Shouldn't the *.so library have the execute permission bits set ?

END
-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit

Reply via email to