Discussion:
final review of fix for #25552
Mike Kupfer
2017-04-25 01:13:19 UTC
Permalink
Attached is the proposed final patch to fix the cl problem (#25552).
The Lisp is the same as before. What's new are the git comments
(ChangeLog format, plus author attribution and a reference to the bug
number).

I'd very much appreciate it if someone could double-check that I got the
git comments right, especially making sure they accurately describe the
Lisp changes. (Any other suggestions for improvements are welcome,
too.)

thanks,
mike
Bill Wohler
2017-04-25 04:41:40 UTC
Permalink
Thanks, Mike!
From the Git perspective, it would be good to have a one-line summary
separated by a blank line for the benefit of gitk and others. I think
this is also consistent with the Emacs conventions. For example, the
commit message could read:

Don't load cl at runtime (Bug#25552)

* lisp/mh-e/mh-acros.el (defun-mh): Check at runtime, not
compile time, whether the target is bound.
* lisp/mh-e/mh-compat.el: Enable compilation. Pull in
mh-acros at compile time.
Attached is the proposed final patch to fix the cl problem (#25552).
The Lisp is the same as before. What's new are the git comments
(ChangeLog format, plus author attribution and a reference to the bug
number).
I'd very much appreciate it if someone could double-check that I got the
git comments right, especially making sure they accurately describe the
Lisp changes. (Any other suggestions for improvements are welcome,
too.)
thanks,
mike
From 5a9c7c0f1a6a23d34828d613cf4739a44f51f48d Mon Sep 17 00:00:00 2001
Date: Mon, 24 Apr 2017 16:54:19 -0800
Subject: [PATCH] Fix MH-E not to load cl at runtime (Bug#25552)
* lisp/mh-e/mh-acros.el (defun-mh): Check at runtime, not
compile time, whether the target is bound.
* lisp/mh-e/mh-compat.el: Enable compilation. Pull in
mh-acros at compile time.
---
lisp/mh-e/mh-acros.el | 7 +++----
lisp/mh-e/mh-compat.el | 3 +--
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/lisp/mh-e/mh-acros.el b/lisp/mh-e/mh-acros.el
index 0c89efb..d424247 100644
--- a/lisp/mh-e/mh-acros.el
+++ b/lisp/mh-e/mh-acros.el
@@ -90,10 +90,9 @@ defun-mh
"Create function NAME.
If FUNCTION exists, then NAME becomes an alias for FUNCTION.
Otherwise, create function NAME with ARG-LIST and BODY."
- (let ((defined-p (fboundp function)))
- (if defined-p
- `(defalias ',name ',function)
+ `(if (fboundp ',function)
+ (defalias ',name ',function)
(put 'defun-mh 'lisp-indent-function 'defun)
(put 'defun-mh 'doc-string-elt 4)
diff --git a/lisp/mh-e/mh-compat.el b/lisp/mh-e/mh-compat.el
index aae751e..8b01774 100644
--- a/lisp/mh-e/mh-compat.el
+++ b/lisp/mh-e/mh-compat.el
@@ -40,7 +40,7 @@
;; Items are listed alphabetically (except for mh-require which is
;; needed sooner it would normally appear).
-(require 'mh-acros)
+(eval-when-compile (require 'mh-acros))
(mh-do-in-gnu-emacs
(defalias 'mh-require 'require))
@@ -384,7 +384,6 @@ mh-write-file-functions
(provide 'mh-compat)
-;; no-byte-compile: t
;; indent-tabs-mode: nil
;; sentence-end-double-space: nil
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
mh-e-devel mailing list
https://lists.sourceforge.net/lists/listinfo/mh-e-devel
--
Bill Wohler <***@newt.com> aka <***@nasa.gov>
http://www.newt.com/wohler/
GnuPG ID:610BD9AD
Mike Kupfer
2017-04-26 01:32:18 UTC
Permalink
From the Git perspective, it would be good to have a one-line summary
separated by a blank line for the benefit of gitk and others. I think
this is also consistent with the Emacs conventions.
Yes to all that.

The git comments actually are formatted as you describe. But "git
format-patch" takes that first line and uses it as the subject. :-/
Maybe I should just use "git log -p" when posting stuff for review...

$ git log
commit 5a9c7c0f1a6a23d34828d613cf4739a44f51f48d
Author: Mike Kupfer <***@alum.berkeley.edu>
Date: Mon Apr 24 16:54:19 2017 -0800

Fix MH-E not to load cl at runtime (Bug#25552)

* lisp/mh-e/mh-acros.el (defun-mh): Check at runtime, not
compile time, whether the target is bound.
* lisp/mh-e/mh-compat.el: Enable compilation. Pull in
mh-acros at compile time.
Authored-by: Glenn Morris <***@gnu.org>, Noam Postavsky
<***@users.sourceforge.net>

How's the above look?

cheers,
mike
Bill Wohler
2017-04-26 04:20:21 UTC
Permalink
Sweet, thanks!
Post by Mike Kupfer
From the Git perspective, it would be good to have a one-line summary
separated by a blank line for the benefit of gitk and others. I think
this is also consistent with the Emacs conventions.
Yes to all that.
The git comments actually are formatted as you describe. But "git
format-patch" takes that first line and uses it as the subject. :-/
Maybe I should just use "git log -p" when posting stuff for review...
$ git log
commit 5a9c7c0f1a6a23d34828d613cf4739a44f51f48d
Date: Mon Apr 24 16:54:19 2017 -0800
Fix MH-E not to load cl at runtime (Bug#25552)
* lisp/mh-e/mh-acros.el (defun-mh): Check at runtime, not
compile time, whether the target is bound.
* lisp/mh-e/mh-compat.el: Enable compilation. Pull in
mh-acros at compile time.
How's the above look?
cheers,
mike
--
Bill Wohler <***@newt.com> aka <***@nasa.gov>
http://www.newt.com/wohler/
GnuPG ID:610BD9AD
Continue reading on narkive:
Loading...