Hello!

Comments from Paul Cunningham incorporated, second version of webrev is 
available at http://cr.opensolaris.org/~pslechta/findbugs2/

Please review the code.

Thank you in advance!

Petr Slechta


Paul Cunningham wrote:
> Petr,
>
> See comments below from my quick skip through ...
>
> Paul
>
> Petr Slechta wrote:
>>
>> FindBugs utility should be integrated into OpenSolaris 2009.05.
>>
>> I would like to ask you to review my code: 
>> http://cr.opensolaris.org/~pslechta/findbugs/
>
> === Start of Comments ====
>
>
> 1. File copyright statements in all files
>    Change year to 2009
>
> 2. usr/src/cmd/findbugs/install-findbugs
>    Rename this to 'install-sfw' (its more common name)
>
>    Line '105 mkdir ....' put this directory in the Targetdirs
>    file and delete this line. (and delete line 104)
>    Line 106, use _install to install it into the proto area
>
>    Move the lines 70, 60, 97, 103, etc, to near the top of the
>    file.
>
> 3. usr/src/cmd/findbugs/METADATA
>    Add missing fields, see guidelines in ...
>    http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines
>
> 4. usr/src/cmd/findbugs/Makefile.sfw
>    Extract the FINDBUGS_VER= info from the METADATA file (see
>    other recent putbacks for examples). You could also do it for
>    FINDBUGS_HOME=
>
>    Combine lines 33 & 34 ...
>      $(RM) -r ${FINDBUGS_HOME} findbugs-${FINDBUGS_VER}-doc
>
>    You could probably also change ksh93 on line 47 to $(SHELL)
>    ?I think?
>
> 5. usr/src/cmd/findbugs/manpages/findbugs.1
>    Did you write this? If so doesn't it need the CDDL header
>    and Sun copyright stuff added?
>
> 6. usr/src/cmd/findbugs/manpages/sunman-stability
>    Line 20 -    SUNWgrails      ????
>    and line 26 ???
>
> 7. usr/src/pkgdefs/Makefile
>     & usr/src/cmd/Makefile
>    I think you should have added your stuff into these (i
>    can't see them in the webrev)
>
> 8. usr/src/pkgdefs/SUNWfindbugs/pkginfo.tmpl
>    Add version at end of DESC= line, eg ...
>     DESC="........ (1.3.5)"
>    Also be more descriptive of what this pkg is
>
> 9. usr/src/pkgdefs/SUNWfindbugs/prototype_i386
>      & usr/src/pkgdefs/SUNWfindbugs/prototype_sparc
>    Comment line pkg name SUNWgrails ???
>
> === End of Comments ======


Reply via email to