-
Notifications
You must be signed in to change notification settings - Fork 104
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
Get rid of debug hooks shenanigans #1213
Conversation
It's currently horribly broken on CircleCI under Busted, for... reasons. And, surprise surprise, it was originally implemented as... a CI hack. (in 897906d). Plus, I don't think we actually ever set debug hooks in the first place anyway? I fully expect the testsuite to implode on some random new stuff insetad, but, oh, well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably okay, but you could also just say something like debug = {sethook = function() end}
so a human can easily swap it in and out.
AFAIU, nothing is ever actually setting a hook, so I'm not sure it serves
any purpose now, even at the best of times ;).
…On Sun, Oct 11, 2020, 20:37 Frans de Jonge ***@***.***> wrote:
***@***.**** approved this pull request.
Probably okay, but you could also just say something like debug =
{sethook = function() end} so a human can easily swap it in and out.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1213 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA3KZR4HPG6QFWHIPOIQWTSKH3NXANCNFSM4SL4JAEQ>
.
|
Also, C BB now ;p.
…On Sun, Oct 11, 2020, 20:38 NiLuJe ***@***.***> wrote:
AFAIU, nothing is ever actually setting a hook, so I'm not sure it serves
any purpose now, even in the best of times ;).
On Sun, Oct 11, 2020, 20:37 Frans de Jonge ***@***.***>
wrote:
> ***@***.**** approved this pull request.
>
> Probably okay, but you could also just say something like debug =
> {sethook = function() end} so a human can easily swap it in and out.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#1213 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAA3KZR4HPG6QFWHIPOIQWTSKH3NXANCNFSM4SL4JAEQ>
> .
>
|
Depends a bit, if they're in Really Useful™ positions I tend to prefer keeping such things around for the future over cleaning it up, so to speak. ymmv I suppose |
It's currently horribly broken on CircleCI under Busted, for... reasons.
And, surprise surprise, it was originally implemented as... a CI hack.
(in 897906d).
Plus, I don't think we actually ever set debug hooks in the first place
anyway?
I fully expect the testsuite to implode on some random new stuff instead,
but, oh, well.
This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)