Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 wart@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|wart@xxxxxxxxxx |mmaslano@xxxxxxxxxx Flag|fedora-review? |fedora-review- ------- Additional Comments From wart@xxxxxxxxxx 2007-02-08 23:06 EST ------- 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 -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review