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