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: expect https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225743 ------- Additional Comments From ruben@xxxxxxxxxxxxxxxx 2007-02-07 11:04 EST ------- Hi Miloslav My comments inline: >> * The package should contain the text of the license > It does, in /usr/share/expect-*/README: I missed that one, thanks for pointing it out. >> * Duplicate BuildRequires: tcl-devel (by tk-devel), libX11-devel >> (by tk-devel), autoconf (by automake) > Because expect explicitly refers to tcl-devel and autoconf it IMHO should > BuildRequire it explicitly; this is unrelated to the question whether e.g. > automake depends on autoconf. Agreed. > BuildRequires: libX11-devel removed, expect doesn't directly refer to libX11. Thanks. >> * Please use {?dist} in the Release tag > I'd rather not; as long as each Fedora release uses a different NEVR, the > dist tag is IMHO just useless clutter. There are a number of pros and cons for the disttag. One pro is that it makes it easier to do mass rebuilds (for example for FC7-test1). You're not required to change it, of course, but please reconsider it. Some more pros (and cons) at http://fedoraproject.org/wiki/PackagingDrafts/DisttagsForRawHide >> * Can you use make DESTDIR instead of make INSTALLROOT? > No, Makefile.in doesn't support this aspect of GNU coding standards and > the variable is called INSTALL_ROOT. Ok. Can you let me know when you've updated the spec in cvs? I'll have another look then. Thanks, Ruben -- 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