Hi Paul,

Comments inline.

Paul Cunningham wrote:
> George,
> 
> This mainly looks okay to me from my quick skip through, see comments 
> below ..
> 
> Paul
> 
> George Vasick wrote:
> 
>>
>> I am requesting a code review for the upgrade of binutils from 2.15 to 
>> 2.19.
> 
>>
>> The webrev is available at the following location:
>>
>> http://cr.opensolaris.org/~gvasick/6838585/
> 
> 1. usr/src/cmd/binutils/Makefile.sfw
>      & Makefile.master
>    Maybe the paths for /usr/gnu & /usr/gnu/share/man should be put
>    into Makefile.master with the equivalent of CONFIGURE_OPTIONS so
>    that they can be used by all GNU pkgs,
>    eg. something like ...
> 
>      CFGGNUPREFIX=   /usr/gnu
>      CFGGNUMAN=         $(CFGGNUPREFIX)/share/man
> 
>      CONFIGUREGNU_OPTIONS =    --prefix=$(CFGGNUPREFIX)
>      CONFIGUREGNU_OPTIONS +=    --mandir=$(CFGGNUMAN)
> 
>      Line ...
>        66                 --infodir=/usr/share/info \
>      could be ...
>                 --infodir=$(CFGINFO)

I pinged the gatekeepers on this one.  If they decided to go this 
direction, I will update the makefile to match.

> 
>      Cosmetic: Align line 60 with the other stuff

Done.

> 
>      Lines 111-119, could you have used protofix to do this?

I tried it out but the result was null files.  The sed script requires 
separate source and destination files.

> 
> 2. usr/src/pkgdefs/SUNWbinutils/Makefile
>    Did you need to change this?, I don't think you have
>    done anything real to it.

Removed.

> 
> 3. usr/src/pkgdefs/SUNWbinutils/copyright
>    Is 'Sun disclaimer' correct ?

Yes.  The text was taken directly from the legal review.

> 
>    Add source-owner copyright lines

I think I have that on line 5 with one correction, 1983 in place of 
1987.  Did I miss something else?


Thanks,
George

> 
> END

Reply via email to