-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
snekbox wrapper #30
Comments
I'll be taking care of this, discussed here |
@ChrisLovering We can use all the methods present in site's The only thing that bugs me is that it'll be coupled to Update1 Update2 |
@ChrisLovering a couple of questions here
I'm struggling to see why would snekbox wrapper contain paste service utilities, shouldn't it have its own wrapper ?
I noticed that we don't use snekbox to eval code in the Internal Cog, is there a particular reason for that ? If not, wouldn't we want to migrate the Internal Cog's eval function to rely on snekbox ? Last thing, I don't see any Latex cog in our code base, so I wanted to make sure whether I'm missing something or not. @MarkKoz We could use your input here since I see you're the one who mentioned this in #85 |
This was in the context of which modules should be ported over, so it doesn't mean that the paste utils have to be part of the snekbox wrapper.
There used to be one in Sir Lancebot, but maybe it got removed or disabled due to issues.
snekbox is for evaluating code in a sandboxed environment. Internal eval is for evaluating code within the context of the bot, so we do not want it sandboxed.
Yes, this https://github.com/python-discord/bot/blob/main/bot/utils/services.py |
Alright, thanks @MarkKoz for clarifying.
In the meantime, I suggest editing the original comment that Chris has made about what should be ported over for better context to future readers |
Though interal eval does not use snekbox, there is likely still some overlap in code. I imagine we can have a base class with common features, and we can also take the opportunity to make the eval commands behave more consistently with each other. Internal eval is used in pretty much every bot, so we can just straight up port that command over I believe (modifying it to use the base class described above). Snekbox will only ever be in one bot, so I'm not sure why this issue is named "snekbox wrapper". @ChrisLovering can you clarify? |
Yea, i did not imagine this to be a wrapper for the snekbox API itself, as it is already very trivial, but rather a wrapper for the logic involved in |
Re. |
This should include:
The text was updated successfully, but these errors were encountered: