[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: scid (updated package)



On Tue, May 12, 2009 at 6:04 PM, W. van den Akker <listsrv@wilsoft.nl> wrote:

> - dget http://mentors.debian.net/debian/pool/main/s/scid/scid_3.7.3-1.dsc

Here is a review:

Some warnings from dpkg-shlibdeps, please investigate and forward
upstream if appropriate:

dpkg-shlibdeps: warning: dependency on libdl.so.2 could be avoided if
"debian/scid/usr/games/tcscid debian/scid/usr/games/tkscid" were not
uselessly linked against it (they use none of its symbols).
dpkg-shlibdeps: warning: dependency on libgcc_s.so.1 could be avoided
if "debian/scid/usr/games/scmerge debian/scid/usr/games/scidlet
debian/scid/usr/games/tcscid debian/scid/usr/games/scidt
debian/scid/usr/games/pgnscid debian/scid/usr/games/tkscid" were not
uselessly linked against it (they use none of its symbols).
dpkg-shlibdeps: warning: dependency on libX11.so.6 could be avoided if
"debian/scid/usr/games/tkscid" were not uselessly linked against it
(they use none of its symbols).

You should never, ever hard-code a dependency on zlib1g, that is
handled by the shlibs mechanism.

README.Debian references the pkg-scid project on alioth. If you aren't
using that, I suggest removing it from README.Debian. collab-maint
might be a better place for a VCS in any case.

Why the change from tex-chess to texlive-games? Why the addition of
oss-compat? Please document the change and reason for it in the
changelog.

Why the difference between debian/compat and the debhelper dependency
in debian/control? Please mention the change from compat level 4->5 in
the changelog too.

Please put proper attribution in the patches instead of
wvdakker@localhost. I also suggest you should set
NAME/DEBNAME/DEBFULLNAME and EMAIL/DEBEMAIL/REPORTBUGEMAIL in your
environment variables.

You repacked the upstream tarball but there is no indication that you
have done so, how it was done or why it was done. These are usually
documented in the version number, debian/rules get-orig-source and the
README.source file. Please see debian-policy/devref for information
about how to do that. Please also get those changes upstreamed, or
obsoleted upstream.

Since you have repacked the upstream tarball, you should also remove
the embedded zlib copy. Please ask upstream to remove the embedded
zlib copy.

polyglot is already in Debian, I think the embedded code copy in
src/polyglot should be removed. Please talk to upstream about why it
was added there and get it removed.

Looks like there is something similar going on with 'crafty', which is
non-free. Please investigate which files these are and have them
removed from the package. Please file an RC bug too if the version in
the archive has the same issue. Fixing it in stable/oldstable is best
dealt with by removal from there, squeeze/sid can be dealt with by a
sid upload.

Please improve the manual page a bit and forward it upstream.

Please get upstream to add a FreeDesktop menu file since the Debian
menu file is Debian-specific and doesn't work in Debian GNOME (by
default) or LXDE.

Please talk to upstream about the patches. Some parts looks like they
can be replaced by passing things to make:

make BINDIR=/usr/games SHAREDIR=/usr/share/scid TCL_INCLUDE=-I/usr/include...

Not entirely sure, but I think the same can be said for the make install patch.

Since you don't use the embedded zlib copy, no need to patch it really.

There is a bug in the second patch:

+	rm -rf $(DESTDIR) $(SHAREDIR)/bases

The upstream clean rule in the Makefile should not touch anything in DESTDIR.

The debian/TODO file says "Compile without warnings on GCC 4.3" when
it already does so.

Some minor changelog style/wording fixes needed:

scid (3.7.3-1) unstable; urgency=low

  * New maintainer
  * New upstream release (Closes: #487771)
    - Fixed bug in ECO browsing (Closes: #401210)
    - Fixed copy/paste bug (Closes: #391385)
  * Updated to tcl8.5 and tk8.5 (Closes: #465788)
  * Include the dpatch system.
  * Added in Changelog some suggested packages : Toga, Phalanx.
  * Updated standard version.
  * Added Homepage field to Control.
  * Added Watchfile.
  * Cleanup rules file. Removed indep/dep build commands which are not used.

 -- Willem van den Akker <wvdakker@wilsoft.nl>  Tue, 21 Apr 2009 07:48:31 +0200

-- 
bye,
pabs

http://wiki.debian.org/PaulWise


Reply to: