Steve,

Yes that looks better.

And it still all looks mainly good, just a few minor comments below. ...

Paul

=============================

Comments:

1. copyright message
I don't think you really needed to change the copyright date in the 
files you haven't changed otherwise, eg. the Makefile.sfw files and 
others. But as you have done them that's fine by me.

2. src/pkgdefs/SFWrpmS/depend
Doesn't the build use the default depend for this, ie, this shouldn't be 
in the src repository. See it SFWrpmS/Makefile

=============================

Steven Christensen wrote:
> Paul -
> 
> I have discovered why the webrev -S was giving strange
> information.  The original source code in my usr/src/cmd
> directories etc were owned by me (steve) and by root
> since I had done edits before as steve and later as root.
> It seems that webrev -S does not give the correct output
> in such a case.  When I changed all the files to be owned
> by steve, webrev -S generated the ---new and ---old output
> correctly.  The new webrev is at the same place
> 
> http://companion.sunfreeware.com/downloads/rpmtetexwebrev/
> 
> and the new code is in
> 
> http://companion.sunfreeware.com/downloads/rpmtetex.tar.bz2
> 
> See if you think this is now OK.   Thanks.
> 
> Steve C.
> 
> 
> On 9/28/06, *Paul Cunningham * <paulcun at talk21.com 
> <mailto:paulcun at talk21.com>> wrote:
> 
>      > I am not sure that webrev -S is doing things correctly.
>      > Am I missing something?
> 
>     To be honest I can't remember how a 'deleted' file should look in the
>     webrev, but its probably of the form ...
> 
>        "------ ------ ------ Old --- src/pkgdefs/SFWrpm/prototype_i386"
> 
>     So as long as you are sure those file which have been deleted, but still
>     exist (as they are auto-generated now), do not get committed/putback
>     (and are actually deleted from the repository) all should be well (as
>     yours seem to have been).
> 
> 
>     Just wondering ...
>     SVN, should the changed files have svn revision info status in them
>     instead of the old out-of-date sccs revision info?
> 
>     Paul
> 
>     Steven Christensen wrote:
>      > Paul -
>      >
>      > I am not sure that webrev -S is doing things correctly.  If I go into
>      > the various directories, I get, when running svn status --verbose
>      >
>      > In usr/src/cmd/tetex
>      >
>      >                78       55 steve        .
>      >                78        1 root         tetex-src-2.0.2.tar.gz
>      > A               0       ?   ?           README.SFWtetex.tmpl
>      >                78        1 root         DISTDIRS.sfw
>      > M            78       55 steve        install-sfw
>      >                78        1 root         DISTFILES.sfw
>      > D             78       55 steve        README.SFWtetex
>      >                78       55 steve         Makefile.sfw
>      >                78       55 steve        README.sfw
>      >                78        1 root         tetex-texmf-2.0.2.tar.gz
>      >                78        1 root         EXFILES.sfw
>      >
>      > in /usr/src/cmd/rpm
>      >
>      >                78       47 steve        .
>      >                78       47 steve        patch
>      >                78        1 root         rpm-4.1.tar.bz2
>      >                78        1 root         DISTDIRS.sfw
>      > M              78        1 root         install-sfw
>      >                78        1 root         DISTFILES.sfw
>      >                78        1 root         rpm-all-patches
>      >                78        1 root         README.SFWrpm
>      >                78       46 steve        Makefile.sfw
>      >                78        1 root         README.sfw
>      >                78        1 root         EXFILES.sfw
>      >
>      > in /usr/src/pkgdefs/SFWtetex
>      >
>      > ?                                       .make.state
>      >                78       55 steve        .
>      > D              78       55 steve        depend
>      >                78       55 steve        prototype_com
>      >                78        1 root         preremove
>      >                78       55 steve        copyright
>      >                78       55 steve        pkginfo.tmpl
>      > A               0       ?   ?           prototype_sparc.tmpl
>      > A               0       ?   ?           prototype_i386.tmpl
>      > M              78        1 root         postinstall
>      > M              78       55 steve        Makefile
>      > D              78       55 steve        prototype_sparc
>      > D              78       55 steve        prototype_i386
>      >
>      > and finally in usr/src/pkgdefs/SFWrpm
>      >
>      > ?                                       .make.state
>      >                78       55 steve        .
>      > D              78       55 steve        depend
>      >                78       55 steve        prototype_com
>      >                78       55 steve        copyright
>      >                78       55 steve         pkginfo.tmpl
>      > A               0       ?   ?           prototype_sparc.tmpl
>      > A               0       ?   ?           prototype_i386.tmpl
>      > M              78       55 steve        Makefile
>      > D              78       55 steve        prototype_sparc
>      > D              78       55 steve        prototype_i386
>      >
>      > So these look right to me in my working copy of the repository.  What
>      > webrev is generating seems to be a problem to me.  Am I missing
>      > something?
>      >
>      > Your comment about the copyrights is correct and I will fix that.
>      >
>      > Thanks,
>      >
>      > Steve C.
>      >
>      >
>      > On 9/28/06, *Paul Cunningham* < paulcun at talk21.com
>     <mailto:paulcun at talk21.com>
>      > <mailto:paulcun at talk21.com <mailto:paulcun at talk21.com>>> wrote:
>      >
>      >     Steve,
>      >
>      >     Changes mainly look okay to me. See minor comments below ...
>      >
>      >     Paul
>      >
>      >     src/cmd/tetex/README.SFWtetex
>      >        shouldn't these show up as deleted as its now
>     autogenerated from
>      >        the tmpl file? ie. removed from the source repository.
>      >
>      >     src/pkgdefs/SFWtetex/prototype_sparc
>      >     src/pkgdefs/SFWtetex/prototype_i386
>      >        - shouldn't these show up as deleted as they are now
>     autogenerated
>      >          in the Makefile from the tmpl files ?
>      >
>      >     src/pkgdefs/SFWrpm/prototype_sparc
>      >     src/pkgdefs/SFWrpm/prototype_i386
>      >        - and again for rpm
>      >
>      >     src/pkgdefs/SFWtetex/depend
>      >     src/pkgdefs/SFWrpm/depend
>      >        - shouldn't these show up as deleted (from src repository)
>     as its
>      >          now using the default depend file ?
>      >
>      >     all files
>      >        - copyright messages - should you have changed the year?
>      >
>      >
>      >     Steven Christensen wrote:
>      >      > This is a request for a code review for two packages on the
>      >     Companion CD.
>      >      > The rpm and tetex install-sfw files, the protoype_sparc and
>      >      > prototype_i386 files, and a few other files have had
>     solaris 2.9
>      >     entries
>      >      > hardcoded in.  The changes documented in the webrev below
>      >      > take care of this problem so that the level of Solaris is
>     picked up
>      >      > and put into the relevant files where needed.  Some minor
>     fixes
>      >      > taking out unneeded depend files were also done.
>      >      >
>      >      > These changes work correctly in nightly builds and in package
>      >      > installation.
>      >      >
>      >      > The webrev is at:
>      >      >
>      >      > http://companion.sunfreeware.com/downloads/rpmtetexwebrev/
>     <http://companion.sunfreeware.com/downloads/rpmtetexwebrev/>
>      >      >
>      >      > The actual files (to be bunzipped and untarred in usr/src) are
>      >      > in
>      >      >
>      >      > http://companion.sunfreeware.com/downloads/rpmtetex.tar.bz2
>      >      >
>      >      > Please send any comments to companion-discuss and CC to
>      >      > me at steve at smc.vnet.net <mailto:steve at smc.vnet.net>
>     <mailto:steve at smc.vnet.net <mailto:steve at smc.vnet.net>>
>      >     <mailto:steve at smc.vnet.net <mailto:steve at smc.vnet.net>
>     <mailto:steve at smc.vnet.net <mailto:steve at smc.vnet.net>>>.
>      >      >
>      >      > Thanks,
>      >      >
>      >      > Steve Christensen
>      >      >
>      >      >
>      >      >
>      >    
>     ------------------------------------------------------------------------
> 
>      >      >
>      >      > _______________________________________________
>      >      > companion-discuss mailing list
>      >      > companion-discuss at opensolaris.org
>     <mailto:companion-discuss at opensolaris.org>
>      >     <mailto:companion-discuss at opensolaris.org
>     <mailto:companion-discuss at opensolaris.org>>
>      >      > http://opensolaris.org/mailman/listinfo/companion-discuss
>     <http://opensolaris.org/mailman/listinfo/companion-discuss>
>      >
>      >
> 
> 

-- 
___________________________________________________________
   Paul Cunningham              Email: paulcun at talk21.com
   Software Engineer            Tel:   01462 685974
   Letchworth Garden City
   Hertfordshire
   SG6 4LH

Reply via email to