-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
websocket-test.el: ‘flet’ is an obsolete macro (as of [emacs] 24.3) #71
Comments
Thanks for reminding me, I fixed this just now, in commit 341ec2b. As to why I use dynamic scope with letf in the first place, it's useful for unit tests. I'm not sure which defvar you are mentioning, though. |
Thanks for reminding me, I fixed this just now, in commit 341ec2b.
Thanks!
As to why I use dynamic scope with letf in the first place, it's [useful for unit tests](https://nullprogram.com/blog/2017/10/27/). I'm not sure which defvar you are mentioning, though.
That makes sense, and I have to confess I'm not familiar with the unit
test-specific pros and cons of dynamic scope (other than the
nullprogram.com .
Most of them, but of course everyone is entitled to use to their own
style :-) The reasons for my concern are: 1. I've noticed a higher than
average incidence of subtle and/or bizarre and indeterminate CI failures
in packages that continue to prefer dynamic binding by default 2. By the
time I started to get serious about elisp, lexical scope was the strongly
recommended best practise...and because of this I have to confess
potential bias.
At this point I'm not sure if #72 is a normal bug or one of those tricky
(1) bugs.
Cheers!
Nicholas
|
P.S. There may also be upstream pressure to transition https://git.savannah.gnu.org/cgit/emacs.git/tree/etc/NEWS?h=emacs-27.1#n326 and I suspect that might be the cause of the CI regressions I've noted in various packages that use dynamic binding. We'll soon switch to Emacs 27.1 in Debian, but hopefully my worries are unfounded ;-) |
Hi,
Prioritise this however you'd like. I'm only reporting it because someday the obsolete macro may be removed.
websocket-test.el: ‘flet’ is an obsolete macro (as of 24.3); use either ‘cl-flet’ or ‘cl-letf’; use either ‘cl-flet’ or ‘cl-letf’. It seems like it work be orthogonal to converting websocket-functional-test.el and websocket-test.el to also use lexical-binding.
P.S. I'm also curious why you're using explicit dynamic binding by using defvar instead of setq. To be fair, my bias is "use lexical, whenever possible" ;-)
Thanks,
Nicholas
The text was updated successfully, but these errors were encountered: