fedora-review denied: [Bug 226479] Merge Review: tcl

bugzilla at redhat.com bugzilla at redhat.com
Fri Feb 9 04:06:11 UTC 2007


Bug 226479: Merge Review: tcl
Product: Fedora Extras
Version: devel
Component: Package Review

Wart <wart at kobold.org> has denied Wart <wart at kobold.org>'s request for
fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479

------- Additional Comments from Wart <wart at kobold.org>
Since Tcl is currently flip-flopping between 8.5a5 and 8.4, I'll start
with some of the low-hanging fruit in the spec file, and return to more
version-specific issues once it settles down.

* Source1 should no longer be necessary now that tk has been split out
  into a separate package.  This will greatly reduce the size of the
  src rpm.

* Removing Source1 will let you collapse the build directory by one
  level, since the tk sources no longer get unpacked alongside the tcl
  sources.  Make sure to adjust the setup and patch commands to reflect
  this new structure.

* Source2 contains the html sources.  Aren't these already built from the
  Tcl sources, or do they have to come from a separate tarball?  If
  they must come from a separate tarball, it would be nice to include
  a pointer to the original url, or commands used to generate the html
  files.

* BuildRequires: sed not necessary.  Likewise, I suspect that
  BuildRequires: man isn't needed either.

* The Version: and URL: tags for the subpackages are redundant.  They
  are copied from the main package if not found.

* In %build, ls %{_tmppath} is pointless.  Delete it.

* Move the 'make test' conditionals to a separate '%check' section instead of
  putting it in %build.

* %install contains a 'make html' command that appears to be building the
  html documentation.  This should be in %build, unless it requires
  Tcl to be installed before running.  Even if that's the case, it
  might be possible to run Tcl from the build directory in order to generate
  the html docs.  Either move 'make html' to %build, or document in the
  spec file why it needs to be in %install.

* The backwards compatible symlink might be better addressed by creating
  explicit directories. As it is now, it creates a link from /usr/lib to
  /usr/share/tcl8.5 on 64-bit systems, and doesn't create and own
  /usr/lib/tcl8.5 at all.
  See bug #227200 for one possible workaround.

* The %post and %postun sections should be replaced by one-liners:
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig




More information about the Fedora-package-review mailing list