David,

New webrevs have been uploaded to:
http://cr.grommit.com/~dermot/

Further comments inline.


David.Comay at sun.com wrote:
>> I'm merged with some recent changes and prepared a final
>> draft of the webrevs for the bison and gm4 changes before
>> I submit the WebRTI and do my putbacks:
>> http://cr.grommit.com/~dermot/
> 
> webrev-bison-03
> ===============
> 
> One comment concerning "webrev" or perhaps your wx active file.  It
> looks like you have the CR listed as "6521207," (note the trailing
> comma) rather than "6521207".


I didn't realize the sccs comment would be used
like this.  I've re-done all the changes, removing
the comma after the bugid.


> usr/src/cmd/bison/install-sfw
> usr/src/cmd/bison/install-bison
> 
>     The changes between these two files are fine but I want to
>     confirm that you did a "workspace filemv" (or "sccsmv") between
>     install-sfw to install-bison.  It sort of looks like you did
>     this (the SCCS id incremented) but the file name of
>     install-bison looks like you froze the ident string before
>     doing the move.
> 
>     Also, "install-bison" has a bug of other CRs listed in the
>     webrev.  It's unclear to me why you have them there (they
>     definitely should not be there) although again perhaps this is
>     confusion on the part of webrev and the rename taking place.


Yes - I did a "workspace filemv".  It looks like a bug
in webrev that displays the full history of a file that
has been renamed, instead of only the most recent change.
If you still think it's wrong, how can I fix it?


> usr/src/pkgdefs/SUNW0sfw/prototype_com
> 
>     It looks like there are a few duplicate entries
> 
>     Lines 216-219 - Duplicates for "da", "de", "es" and "et".
> 
>     Line 286 - Duplicate for "fr".
> 
>     Lines 380-382 - Duplicates for "hr", "id" and "it".
> 
>     Line 416 - Duplicate for "ja".
> 
>     Lines 441-443 - Duplicates for "nl", "pt_BR" and "ru".
> 
>     Lines 508-509 - Duplicates for "sv" and "tr".


Sorry - these must have slipped in during the most
recent filemerge.  Fixed now.


> usr/src/pkgdefs/SUNWbison/prototype_com
> 
>     Could you please sort the list of entries based on file name
>     (third field?)


OK - done.


> usr/src/pkgdefs/SUNWbisonS/prototype_com
> 
>     Could you please sort the list of entries based on file name
>     (third field?)


OK - done.


> 
> usr/src/pkgdefs/SUNWsfinf/prototype_com
> 
>     As these are in blocks based by component, could you just sort
>     that initial block (since "sfw" comes before "share".)


OK - done.


> webrev-gm4-05
> =============
> 
> usr/src/cmd/gm4/install-sfw
> usr/src/cmd/gm4/install-gm4
> 
>     Same comment as above concerning bison's install-* files.
> 
>     And same comment concerning extraneous CRs listed for
>     "install-gm4".
> 
> usr/src/pkgdefs/SUNWgm4S/prototype_com
> 
>     Could you please sort the list of entries based on file name
>     (third field?)


OK - done.


> usr/src/pkgdefs/SUNWsfinf/prototype_com
> 
>     As these are in blocks based by component, could you just sort
>     that initial block (since "sfw" comes before "share".)


OK - done.


> One final meta-comment - from the /usr/gnu case, conflicting commands
> like GNU m4 are supposed to have the "standard" command's man page
> augmented to reflect the language in section 2.7 of the umbrella case.
> Please be sure to file a CR under solaris/manpage/section1 for this to
> be done for m4(1).


Will do.

- Dermot


> dsc

Reply via email to