Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=484049 --- Comment #14 from Alan Dunn <amdunn@xxxxxxxxx> 2009-07-29 21:04:16 EDT --- I made the changes you suggested. New SRPM version is just 3 -> 4, while spec is in the same location as before. Minor comments below. (In reply to comment #13) > Sorry to take so long with this. I've been out on vacation for the last 2 > weeks. > > Two more changes are needed. First, patch comments should appear in the header > rather than on the %patch invocations in the %prep section. See > https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment. Noted and changed. > Second, the handling of patch2 is broken in two ways. First, it isn't inserted > into the source RPM when building on Fedora <= 11. Indeed, the link you gave > in comment #12 leads to a source RPM without the patch. Unconditionally list > the patch, so that it is always present in the source RPM. Second, choosing to > apply the patch based on Fedora release number will break soon; I'm preparing > to push XEmacs 21.5.29 to F-11. The patch I sent should compile and run > successfully on all versions of XEmacs; the point of the wrappers was to hide > the differences between Emacs/older XEmacs and newer XEmacs. I think you can > drop the conditionals and the BR on xemacs >= 21.5.29 and everything should > work. I was trying to only patch things in the versions where I knew things wouldn't work without the patch. (As you point out, my solution would really also necessitate rebuilding the SRPM per distro version, which is bad.) I originally was trying to patch by xemacs EVR available in the distro, but I didn't know how to do this (I started a thread on Fedora-devel about this). Ultimately it's probably easier to apply the patch everywhere as you suggest. > Everything else looks fine. Make those changes and I'll approve it. Thanks > for your hard work. I may be hard to contact for a day or two (moving to a new place half-way across the US tomorrow), but I'll try to be as responsive as possible if there are any further issues. Thanks for reviewing this. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review