[Bug 484049] Review Request: emacs-common-proofgeneral - Emacs mode for standard interaction interface for proof assistants

[bugzilla at redhat.com]

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 at gmail.com>  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.