[companion-discuss] Code Review Request: Fixes to rpm and teTeX packaging
Steven Christensen
sunfreeware at gmail.com
Fri Sep 29 01:09:35 PDT 2006
Paul -
In fact, I had to add the CDDL to the files and so technically,
the Copyright needed to be changed, which I have.
I removed the reference to the depend file that was not
supposed to be in the svn list.
The links have been updated.
Steve C.
On 9/29/06, Paul Cunningham <paulcun at talk21.com> wrote:
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss-beta1.opensolaris.org/pipermail/companion-discuss/attachments/20060929/19c4f3d5/attachment-0002.html
More information about the companion-discuss
mailing list