module: fix decorator multiple-use inconsistency #1632
Conversation
There was a problem hiding this comment.
I nitpicked grammar per usual, but there are some other things in here.
Notably, I did not review the tests yet. (My Sopel bot will remind me to look at them tomorrow.)
I'm also looking at the fix for what started this whole thing—the @url decorator—and wondering if we can't support multiple decorations without adding extra stack frames. Since, as I understand it, each additional time @url is called, it will wrap a new helper() around the previous one(s).
Happy to be wrong about that, but if I am reading it right I'd like to find a more efficient way to achieve the same goal than nested wrapper functions.
These decorators can be properly called many times, with many arguments: * rule * url * commands * nickname_commands
Exirel
force-pushed
the
decorator-consistency
branch
from
524a5a2
to
3a7d25e
Compare
We can. I wanted to be sure that it worked first, and I'm glad you could review this first. I'll work on that next. |
|
@dgw done |
There was a problem hiding this comment.
Looked over the tests before my Sopel bot even reminded me to! 👍
Having reviewed the diff of the third commit (removing the wrapper from @url decorated functions), and seeing that the new tests for @url pass, I say we're good, after taking a pass through loader and bot for good measure to check how url_regex is handled.
currency: switched crypto api and combined rates to allow cross-lookup
Not much to say: fix [main part of] #1619
(editorial by @dgw so GH won't close the other issue)