#dev-log

1 messages · Page 8 of 1

regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
mbaruh
[python-discord/bot] New branch created: legacy\_role
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
wookie184
[python-discord/bot] branch deleted: legacy\_role
oak estuaryBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
wookie184
[bot:main] 5 new commits

ecb835a Make the infraction message mention @ModMail - swfarnsworth
b13025a Update bot/exts/moderation/infraction/_utils.py

  • swfarnsworth
    f10325d Merge branch 'main' into swfarnsworth-modmail-a... - swfarnsworth
    9e18532 Merge branch 'main' into swfarnsworth-modmail-a... - wookie184
    44b2b2a Merge pull request #2374 from python-discord/sw... - wookie184
#
wookie184
[python-discord/bot] branch deleted: swfarnsworth\-modmail\-account\-mention
odd spireBOT
regal archBOT
oak estuaryBOT
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
ionite34
[python-discord/bot] New review comment on pull request #2326: Support \`eval\` \(snekbox\) file system and attachment display

We would still need the successful files though, or else we don't know which of the files in result.files is safe to send. If we accept strings we'd return a string as well, and then we'd need to iterate once more for each result.files to check if the path is in the blocked files, and risk any Path normalization mismatch, as well as needing to use pathlib / os to get the suffixes of string paths to display as errors.

I've attempted generalizing this but it feels more complex and mor...

odd spireBOT
regal archBOT
odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
#
n0Oo0Oo0b
[python-discord/bot] New comment on pull request #2402: reply to user when using \!eval/\!timeit command instead of pinging

bot/issues/2382

Just needed to change ctx.send to ctx.reply and remove the author.mention.

What if the response gets redirected to a different channel? @redirect_output() doesn't change the behavior of ctx.reply() so the result won't be redirected when it should.
It may be worth considering having @redirect_output() add an extra bool attribute (maybe redirected?) for whether or not the output was redirected or maybe even replacing the ctx.reply() function to send a mess...

#
e1pupper
[python-discord/bot] New comment on pull request #2402: reply to user when using \!eval/\!timeit command instead of pinging

bot/issues/2382
Just needed to change ctx.send to ctx.reply and remove the author.mention.

What if the response gets redirected to a different channel? @redirect_output() doesn't change the behavior of ctx.reply() so the result won't be redirected when it should. It may be worth considering having @redirect_output() add an extra bool attribute (maybe redirected?) for whether or not the output was redirected or maybe even replacing the ctx.reply() function to send a m...

oak estuaryBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
HassanAbouelela
[python-discord/bot] branch deleted: reusable\-status\-embed\-workflow
#
shtlrs
[python-discord/bot] New review comment on pull request #2400: Reusable status embed workflow

This has been discussed internally, but for people who might bump into this, here's what has been discussed:

We had the continue-on-error option set because we always need the first job to run entirely in order for the second job to trigger and that's because the second depends on the first.

However, it would be a waste to run future steps knowing they would fail if the previous ones did.

The solution for this was to add an if statement that will always make the second one run.

odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/.github] branch deleted: tagger\-workflow
oak estuaryBOT
odd spireBOT
regal archBOT
#
shtlrs
[python-discord/.github] Pull request opened: #17 Add status\_embed workflow

As a continuation to this PR, we now want all of our repositories that need the status_embed workflow to use a centralized reusable workflow.

We've decided to add it to this repo and have it referenced by all of our other repositories that need to trigger it upon linting, building, etc.

NOTE
the GHA_WEBHOOK_TOKEN secret needs to be part of this repository in order for it to be reusable all over.

regal archBOT
#
shtlrs
[python-discord/bot] New branch created: test\-workflow
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
#
shtlrs
[python-discord/bot] New branch created: revert\-2400\-reusable\-status\-embed\-workflow
odd spireBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
#
shtlrs
[python-discord/bot] Pull request opened: #2408 \[POC\] Scaffold server config via a bootstrap script

The purpose of this PR is to concretize Scale's suggestion to have a bootstrap script that talks to Discord's API, then populate a server environment.

Later on, we will rely on Pydantic to load the config classes based on that newly populated environment file, which we can later use for in out bot.

For simplicity, and only for demo purposes, the previous config classes have been wiped off, and only the roles & channels ones were left, with 2 attributes tops each.

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/bot] branch deleted: revert\-2400\-reusable\-status\-embed\-workflow
odd spireBOT
regal archBOT
oak estuaryBOT
odd spireBOT
regal archBOT
odd spireBOT
night lilyBOT
#
Sir Lancebot

Connected!

oak estuaryBOT
odd spireBOT
odd spireBOT
odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
#
hedyhli
[python-discord/site] New comment on pull request #626: feat: 'Edit on GitHub' for content articles

Any thoughts on how the behaviour on mobile could be improved?

It should wrap so the whole thing is to the next line, like I mentioned in the description. Either same line as sub-articles or each one their own line.

At the moment sub-articles seems to be wrapping to fit on the same line as the title. I doubt this works when title is longer (for pages other than the contributing index page) even before adding the edit on github like, so it would probably be best to have them both take ...

regal archBOT
odd spireBOT
regal archBOT
#
mbaruh
[python-discord/bot] New review comment on pull request #2390: New Filtering System

This will have to assume that every antispam filter has an extra setting called interval (holds now, but isn't logically guaranteed), and the attribute being summed is different every time, so you either exclude the fourth line, or send a function to fetch the right value from each message. I'll think about it, but at first glance trying to generalize it seems clumsy.

regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/sir-lancebot] New branch created: deprecate\-bookmark\-text\-command
odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/sir-lancebot] Pull request opened: #1211 Deprecate bookmark text command

Description

This deprecates the text-based bookmark command, now that we have the context menu command.

Any users that invoke the old text-based command will be told how to use the new context menu command in sir-lance-playground.

This also simplifies a lot of the implementation of the bookmark cog.

Did you:

  • [x] Join the Python Discord Community?
  • [x] Read all the comments in this template?
  • [x] Ensure there is an issue open, or ...
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
#
wookie184
[python-discord/bot] Pull request review submitted: #2402 reply to user when using \!eval/\!timeit command instead of pinging

We need to handle the case where the message is deleted before the bot sends the response. Currently an error is raised:
image
To reproduce, run !e import time;time.sleep(5) and immediately delete your message.

There are three main options I can think of for handling this case:

  1. Catch the error and send no response
  2. Catch the error and resend the message with a ping instead o...
regal archBOT
#
wookie184
[python-discord/bot] Pull request review submitted: #2403 Migrated \`\!tags\` command to slash command \`/tag\`

I'm currently getting an error when running this if there are too many matches, we should probably trim the results in these cases:

bot-bot-1          | 2023-02-19 15:45:18 | asyncio | ERROR | Task exception was never retrieved
bot-bot-1          | future: <Task finished name='CommandTree-invoker' coro=<CommandTree._from_interaction..wrapper() done, defined at /opt/poetry/cache/virtualenvs/bot-TFcQMFAJ-py3.10/lib/python3.10/site-packages/discord/app_commands/tree.py:1089> exception=HTT...
regal archBOT
#
Ibrahim2750mi
[python-discord/bot] New comment on pull request #2403: Migrated \`\!tags\` command to slash command \`/tag\`

I'm currently getting an error when running this if there are too many matches, we should probably trim the results in these cases:

bot-bot-1          | 2023-02-19 15:45:18 | asyncio | ERROR | Task exception was never retrieved
bot-bot-1          | future: <Task finished name='CommandTree-invoker' coro=<CommandTree._from_interaction.<locals>.wrapper() done, defined at /opt/poetry/cache/virtualenvs/bot-TFcQMFAJ-py3.10/lib/python3.10/site-packages/discord/app_commands/tree.py...
#
Numerlor
[python-discord/bot] New comment on issue #2385: Apply pydis\_core\.utils\.cooldown\.block\_duplicate\_invocations decorator to commands where repeated invocations in a short time are redundant

Sure, but I'm currently a bit busy so if anyone else is interested they're free to do so.

On 02/19/2023 5:07 PM CET wookie184 @.***> wrote:

This sounds sensible, are you interested in opening a PR @Numerlor https://github.com/Numerlor?

—
Reply to this email directly, view it on GitHub https://github.com/python-discord/bot/issues/2385#issuecomment-1436027007, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGFP55DKX4YTH7JTS42O6HLWYJAKPANCNFS...

#
wookie184
[python-discord/sir-lancebot] New review comment on pull request #1211: Deprecate bookmark text command

The message redirecting the user is about the same size as the information message itself, so I feel it would make more sense to just send the information directly to the channel where it's invoked- it doesn't seem too noisy.

(There's also currently a slightly weird case currently when using the command in sir_lancebot_playground as it sends the info message and then an embed directly below linking to the message above.

odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
ichard26
[python-discord/site] New branch created: flip\-timeline
#
ichard26
[python-discord/site] Pull request opened: #879 Reverse timeline entries so newest entries come first

We agreed internally that this would look better, and especially as I add more entries.

This was automated using this script:

import dataclasses, sys, textwrap
from pathlib import Path

import bs4


@dataclasses.dataclass
class TimelineEntry:
    title: str
    date: str


html_doc = Path(sys.argv[1]).read_text("utf-8")
soup = bs4.BeautifulSoup(html_doc, "html.parser")

entries = []
for item in soup.find_all(class_="cd-timeline__block"):
    entries.append(Ti...
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/site] branch deleted: flip\-timeline
odd spireBOT
regal archBOT
#
Ibrahim2750mi
[python-discord/sir-lancebot] Issue opened: #1213 Misleading docstring and variable names in \`githubinfo\.py\`

Description

on_message function:
https://github.com/python-discord/sir-lancebot/blob/9ecf423268bb3a89e881e4b28e5ee56fe9b33819/bot/exts/utilities/githubinfo.py#L175
https://github.com/python-discord/sir-lancebot/blob/9ecf423268bb3a89e881e4b28e5ee56fe9b33819/bot/exts/utilities/githubinfo.py#L183

Here the on_message function returns more than just issues, it also returns pull requests. So the docstring and the variable names should be changed accordingly.

Possible Solutions

...

regal archBOT
#
Ibrahim2750mi
[python-discord/site] Issue opened: #880 Contributing to Sir Lancebot page missing important env variable

https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/sir-lancebot/#environment-variables This fragment of the page displays needed environment variables to run the bot but it is missing an environment variable for the trashcan emoji, which is needed to run the help command. And as the help command is common to test the bot, its essential. Same in this [page](https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/sir-lancebot/env-var-reference/#general-variabl...

regal archBOT
regal archBOT
#
ChrisLovering
[python-discord/sir-lancebot] branch deleted: deprecate\-bookmark\-text\-command
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
#
wookie184
[python-discord/bot] New review comment on pull request #2403: Migrated \`\!tags\` command to slash command \`/tag\`

We need to be careful to ensure that we don't break normal tag invocation (with just the name of the tag, e.g. !code). This is handled in the error handler here https://github.com/python-discord/bot/blob/main/bot/exts/backend/error_handler.py#L162

For that it makes sense to give no result if a tag wasn't found, although if a user is using the tag get command explicitly I think they should always get a response (e.g. saying "no tag found").

I'm not sure what the best way to make this cha...

#
wookie184
[python-discord/sir-lancebot] New comment on issue #1213: Misleading docstring and variable names in \`githubinfo\.py\`

From GitHub's API's perspective, pull requests are a type of issue https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28#list-issues-assigned-to-the-authenticated-user

Note: GitHub's REST API considers every pull request an issue, but not every issue is a pull request. ...

For user facing (e.g. help command text) I think it's worth listing both, but because this is internal I'm not sure it's worth changing, although maybe it would be more clear, I don't mind.

#
wookie184
[python-discord/site] New comment on issue #881: Contributing to Sir Lancebot\->Envrionment variables\->Description has mistakes

The names of team roles on the server are plural (Admins, Helpers). A reason for this is that it looks better in pings (because when you ping you're pinging multiple people), and also in the member list (because it's a heading for multiple people with the role). I think it would make most sense to be consistent with that.

The codeblock additions sound good.

regal archBOT
#
Ibrahim2750mi
[python-discord/sir-lancebot] New comment on issue #1213: Misleading docstring and variable names in \`githubinfo\.py\`

For user facing (e.g. help command text) I think it's worth listing both, but because this is internal I'm not sure it's worth changing, although maybe it would be more clear, I don't mind.

@wookie184 When I was trying to find the source of this command I overlooked this file because the docstring mentioned 'issues' in the on_messagefunction, I found the source of this through Chris when he redirected me. So it kinda causes confusion.

regal archBOT
#
Ibrahim2750mi
[python-discord/bot] New review comment on pull request #2403: Migrated \`\!tags\` command to slash command \`/tag\`

The reason for using aliases was for ease of user but now as slash commands display all the commands it would kind of get clustered seeing one command take up all the space in the suggestions pop up, like eg: just writing t will take 6(3 for get, 3 for list) space. Also when just typing the user can see the commands and can now, even autofill the subgroup easily. And this is the reason discord has removed support for aliases for slash commands.

regal archBOT
regal archBOT
#
Ibrahim2750mi
[python-discord/bot] New review comment on pull request #2403: Migrated \`\!tags\` command to slash command \`/tag\`

I think it gets my question unanswered, my question was that earlier the tag text command function returned False when no embed was sent but as of now the tag slash command sends an embed either way(if the tag was found and if the tag wasn't found). So, should the function now return True if an embed is sent for no tag found or should it return False when a tag is not found.

Also about the error handler, I have already made changes to it i.e migrated it to handle slash command, th...

odd spireBOT
regal archBOT
#
wookie184
[python-discord/bot] New comment on issue #2394: Migrating \!tag to slash commands

I'm not sure if the intention of this issue is to replace the direct invocation of tags (e.g. through !code), or just the actual !tag command.

I think replacing the actual !tag command makes sense, but not removing the direct invocation. It does not seem like an improvement to go from !code to /tag get code, pressing enter, then having to press enter again after it autocompletes to /tag get codeblock, all with a bunch of discord UI popping up in the process. Having /tag with ...

regal archBOT
regal archBOT
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
regal archBOT
#
Ibrahim2750mi
[python-discord/bot] New review comment on pull request #2403: Migrated \`\!tags\` command to slash command \`/tag\`

Also wookie the !code is already broken as the tag command now uses discord.Interaction and the ctx captured with the message containing !code has its interaction attribute return None, which means the tag command can't be invoked. A possible work around for this would be to have get_command_ctx in tags.py and try_get_tag_ctx in error_handler.py or we have to make the get_command ugly by doing isinstance everywhere.

regal archBOT
#
GDWR
[python-discord/infra] New branch created: pin\-linting
regal archBOT
regal archBOT
#
wookie184
[python-discord/bot] New review comment on pull request #2403: Migrated \`\!tags\` command to slash command \`/tag\`

(sorry i'm sure you know this already, I just wanted to summarise to ensure we're on the same page).

The error handler is used as the individual tag names aren't registered as commands (e.g. !code), so we catch any CommandNotFound errors, and in those cases try and match the name given to a tag (this also allows us to do fuzzy matching that doesn't interfere with other tags).

Currently the !tag get <name> command, and direct !<name> invocation are pretty much exactly the same and ...

regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
#
Resuscitating
[python-discord/snekbox] Issue opened: #169 sandboxing prevents access to \`/proc/\` directory

f"/proc/{getpid()}/fd/*" isn't available, which I think is a bit too harsh of a sandboxing measure in this case. I don't know of any use case where accessing /proc/{getpid()}/ could be abused. I understand not letting it mess with other processes, but you can restrict it to only its own process using symlinks or something similar, so that shouldn't be much of an issue.

Let me know if that idea sounds good! If it does I'll probably submit a PR within the next week or so to implement t...

regal archBOT
#
shenanigansd
[python-discord/sir-lancebot] New branch created: shenanigansd\-patch\-1
odd spireBOT
regal archBOT
#
Preocts
[python-discord/sir-lancebot] New comment on pull request #1214: holidayreact: add alternate spellings to February

As I've been implemented in this, I present for consideration:

https://regex101.com/r/1hw10r/1

Original (with plural hearts):

\b(love(s|lie|lies)?|wuv(s|lie|lies)?|hearts?)\b

Simplified (no uwu)

\b(l(ove|uv)(s|lies?)?|hearts?)\b

Simplified (with basic uwu because fun)

\b((l|w)(ove|uv)(s|lies?)?|hearts?)\b

test set:

love
lovelies
luv
luve
wuve, what is wuv?
baby don't hurt me
loves
lovelies
lovelie
wuvlie
wuvlies
wu...
odd spireBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
HassanAbouelela
[python-discord/bot] New review comment on pull request #2408: \[POC\] Scaffold server config via a bootstrapping script

A lot of the variables here have quite wordy/redundant configurations. For instance, this one uses a field simply to modify the case of the env key (pydantic has a setting to disable case sensitivity, which I believe is the default). This line should just be debug: str = "true", or possibly even debug = True and let pydantic handle the type-coercesion, and infer the type.

regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
shtlrs
[python-discord/bot] New review comment on pull request #2408: \[POC\] Scaffold server config via a bootstrapping script

That's what I did in the first place, but flake8 didn't like that.
Here's what it says

B008 Do not perform function calls in argument defaults.  The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time fun
ction call.  If this is intended, assign the function call to a module-level variable and use that variable as a default value.

Which is why I did that, I can either leave it the way it is or t...

regal archBOT
#
shtlrs
[python-discord/bot] New review comment on pull request #2408: \[POC\] Scaffold server config via a bootstrapping script

That's not actually it. This is there to ensure backwards compatibility with the env variables that might be in production.
The values are not the same, as you can see here we don't define a prefix for this specific class, unlike the rest where it's usually something__

Since in the old setting this was extracted from a pure env variable called BOT_DEBUG, I thought I'd leave it the way it is.
But if we were to change it, we need to make sure that we define them in the prod config as wel...

regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
oak estuaryBOT
#

Doc item doc_item.symbol_id='csv.csvreader.fieldnames' present in loaded documentation inventories not found on site, inventories may need to be refreshed.

odd spireBOT
odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
shtlrs
[python-discord/site] Pull request opened: #885 Config scaffolding guide

This PR needs to be merged with bot#2408

We're currently migrating our entire configuration to rely on Pydantic models in order to represent all of our needed constants.

This removes the YAML configuration files entirely and relies on .env files to store the values of these constants & map them to our constant classes.

This updates the contribution guide to:

  1. Explain how can contribs generate a server config using the serve...
odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
regal archBOT
#
wookie184
[python-discord/bot] New branch created: timeit\-backticks
odd spireBOT
regal archBOT
regal archBOT
#
wookie184
[python-discord/bot] New review comment on pull request #2403: Migrated \`\!tags\` command to slash command \`/tag\`

I don't think there's any point in having the custom _can_run if it doesn't run any checks. I think we should either just run the global checks manually using the (undocumented) self.bot.can_run, or run the checks for the interaction get command (they should both do the same thing since the interaction get command doesn't add any custom checks).

The main useful global check is the one to ensure we're on the guild, there's also one to ensure the command isn't invoked by a bot - i'm not ...

odd spireBOT
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
#
Ibrahim2750mi
[python-discord/bot] New comment on pull request #2403: Migrated \`\!tags\` command to slash command \`/tag\`

I wonder whether it would be better to just have a /tag <name> command, since currently /tag get with no arguments is the same as /tag list, so i'm not sure there's any point in having two commands. One would be shorter to type and simplify the code a little.

I had a conversation about this on discord with @ChrisLovering, he suggested these making the commands like that.

regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
wookie184
[python-discord/site] New review comment on pull request #882: Contributing Page\->Sir Lancebot: update description

Ah, I didn't realise that, I don't think the current proposed text is that clear as it's not clear that it means the output when pasted into discord. We could just mention that they could set it to 🗑️ (use the unicode char directly).

It would be sort of nice to have something on sir-lancebot's side that made it use that by default in development, so users don't need to set it at all (fwiw the reason we have a custom on on the server is just that it looks nicer).

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
shtlrs
[python-discord/bot] New comment on pull request #2408: \[POC\] Scaffold server config via a bootstrapping script

I'm referring to creating Fields and overriding pydantic's default env key creation. I.e, in the line commented on, the diff would be:

- debug: str = Field(env="BOT_DEBUG", default="true")
+ debug = True

And in prod we'll update the BOT_DEBUG key to whatever pydantic resolved it as.

The confusion happened due to the lack of knowledge that we were ready to update the env var keys in prod as well.

Done in 36ebda1d54cb88033e4be2ef399668f0c3664a15

odd spireBOT
regal archBOT
#
wookie184
[python-discord/sir-lancebot] New comment on pull request #1217: Add default unicode emoji for trashcan

We still want to use the custom one on the server, so this would require setting the env variable in production.

An alternative approach would be to use the Client.debug constant, which defaults to true, but is set to false in production. If this is true we would use the unicode default if the env var wasn't set (this wouldn't require any changes to production environment variables)

Do @python-discord/devops have a preference?

odd spireBOT
regal archBOT
#
Ibrahim2750mi
[python-discord/sir-lancebot] New comment on pull request #1217: Add default unicode emoji for trashcan

An alternative approach would be to use the Client.debug constant, which defaults to true, but is set to false in production. If this is true we would use the unicode default if the env var wasn't set (this wouldn't require any changes to production environment variables)

I don't see Client.debug being false anywhere, if anything, we need to set it false by ourselves. Am I right? Or does this happen outside constants.py

odd spireBOT
regal archBOT
#
wookie184
[python-discord/sir-lancebot] New review comment on pull request #1217: Add default unicode emoji for trashcan 
    trashcan = environ.get(
        "TRASHCAN_EMOJI",
        f"\N{WASTEBASKET}" if Client.debug else "![trashcan](https://cdn.discordapp.com/emojis/637136429717389331.webp?size=128 "trashcan")",
    )

Normally an if would probably be nicer, but here ideally we should avoid any extra variables being left over in the namespace so people don't accidentally use them in code or get confused. (alternatively we could prefix the default with a _ and del it afterwards)

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
henri-climber
[python-discord/sir-lancebot] Pull request opened: #1218 fixed embed interaction and randomly choosing a kata

Relevant Issues

Closes #1170

Description

The issue mentioned two problems:

  1. interactions edit the message but still show as failed
    The problem is that discord automatically says the interaction failed if the bot doesn't respond to the interaction. Adding interaction.response.defer() fixes this. (See this stackoverflow discussion: https://stackoverflow.com/questions/71177867/discord-py-error-message-this-interaction-failed)

  2. The same challenge seems to be shown every t...

odd spireBOT
regal archBOT
#
wookie184
[python-discord/bot] New review comment on pull request #2415: Fix timeit commands with backticks after command name

I guess it still works because it would hit the line if new_ctx.command is None: which would probably be True. I think it makes sense to leave it though, since this test isn't meant to test that command so it makes the test more contained.

Another alternative would be to make it so try_run_fixed_codeblock is actually tested with both True and False, but I don't think it's worth it, since the test is already pretty complicated compared to the very simple code that it's actually testing.

regal archBOT
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/bot] branch deleted: timeit\-backticks
oak estuaryBOT
regal archBOT
odd spireBOT
regal archBOT
#
swfarnsworth
[python-discord/bot] New branch created: swfarnsworth\-code\-command\-paste
#
swfarnsworth
[python-discord/bot] Pull request opened: #2417 Add line to \`\!code\` about the pastebin

Something I've observed a lot is people running the !code command to tell users about markdown blocks, only for them to immediately reply saying "my code is too long". This PR adds a new line to the !code tag telling users about the pastebin.

For the sake of brevity, the added line says nothing about how to use the pastebin. If this PR is merged, and users who attempt to use the pastebin after seeing this tag end up finding it confusing, we can revisit the content of this tag.

![ima...

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/bot] branch deleted: swfarnsworth\-code\-command\-paste
oak estuaryBOT
odd spireBOT
regal archBOT
#
wookie184
[python-discord/bot] New branch created: 2302\-activity\-in\-reviews
#
wookie184
[python-discord/bot] Pull request opened: #2418 Take activity into account in talentpool

Closes #2302

  • Only posts review for users who have been active in the past 7 days
  • Remove users who have been inactive in the past 30 days from the talent pool
  • Make the tp list command sort based on when the review will actually happen, and display
  • Send a message in nomination-discussion if a user is automatically removed from the talentpool.

It's not thoroughly tested yet and I will do some more, although I think it's good enough to open and get reviews.

odd spireBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
#
Ibrahim2750mi
[python-discord/sir-lancebot] New review comment on pull request #1148: Add ability to view previous topics with \.topic command

If the embed's title exceed the character limit you directly use the previous_topic which means that it would not append any new topics, so it will stay stuck at those already present topics no matter how many times it is called, because your function just returns the previous_topic when it has reached the character limit. Eg: assume this has exceeded the character limit

1. How to do python
2. How to not do python
3. beep boop

Now even if your function is called it would just retu...

regal archBOT
#
Anonymous4045
[python-discord/sir-lancebot] New review comment on pull request #1148: Add ability to view previous topics with \.topic command

That's certainly an option, but that would change the order of each topic, which could cause some confusion for people unaware of how it works or with responding to a particular prompt. At the moment, the reaction disappears once the limit is reached to dissuade people from trying to get too many prompts. The reason being that someone in the chat should have something to say about one of the four or so prompts.

I think we're also considering moving the topics from the embed title into the m...

odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
#
mbaruh
[python-discord/bot] New comment on issue #2369: Add an IPython mode to eval

I wonder if this needs to actually be a separate option. On the surface we could just replace the behavior of concatenating several blocks into this. Then again not everyone might find ipython intuitive without prior experience. We could make it default to a usual run if there's up to one codeblocks, and an ipython run otherwise. The error coloring is a very nice bonus.

regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
shtlrs
[python-discord/sir-lancebot] New review comment on pull request #1197: Add possibility of deleting a bm with a button

The docs say:

Setting an empty permissions field will disallow anyone except server administrators from using the command in a guild.

By default, upon instantiation, it defaults to none since the callback doesn't have the __discord_app_commands_default_permissions__ attribute, so I'm wondering if that counts as the same ?

If not, i'll just add this then

self.delete_book_mark_context_menu.default_permissions = []
regal archBOT
odd spireBOT
regal archBOT
#
wookie184
[bot:main] 10 new commits

679cb77 Migrated !tags command to slash command /tag - Ibrahim2750mi
1cff5bf Update tests for /tag as of migration to slas... - Ibrahim2750mi
861f5b2 Merge branch 'main' into migration/tag - Ibrahim2750mi
1bd0310 Merge branch 'main' into migration/tag - Ibrahim2750mi
9b98dfe Implement all reviews - Ibrahim2750mi

odd spireBOT
oak estuaryBOT
odd spireBOT
odd spireBOT
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
oak estuaryBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
wookie184
[site:main] 6 new commits

d091b1d Contributing Page->Sir Lancebot: update descrip... - Ibrahim2750mi
a82c4bf Update description of TRASHCAN_EMOJI accordin... - Ibrahim2750mi
7378c28 remove trashcan emoji from required variables - Ibrahim2750mi
72a070d Merge branch 'main' into contributing/sir-lance... - wookie184
394f5db Merge branch 'main' into contributing/sir-lance... - Xithrius

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
Xithrius
[site:main] 4 new commits

8fb13c0 Update pyproject.toml to use dependancy groups - ChrisLovering
d4340fd Don't use the deprecated --without dev in Docke... - ChrisLovering
e954935 Ensure all deps are installed in CI - ChrisLovering
e82a2e1 Merge pull request #847 from python-discord/poe... - Xithrius

#
Xithrius
[python-discord/site] branch deleted: poetry\-groups
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
#
shtlrs
[python-discord/site] New review comment on pull request #885: Update contribution guide with the new configuration scheme

I'm not too sure about that either, because If I understood right, we'll be revamping the test server as well so that it reflects the necessary channels for the bot to work.
AFAIK, we've only condensed the test server channels to avoid the config hell that we used to go through, which will soon change.

What I meant by "this is not an exact mirror" is the fact that not ALL channels from prod are here since they're not needed for the bot to work, but maybe we should work on that rewording, ...

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
oak estuaryBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
#
henri-climber
[python-discord/bot] New comment on pull request #2089: Feat: add reminder snoozing & show new expiry / mention count when editing reminder

It's been a while since this discussion was active but i really like the idea.
I just tested everything but for me there seems to be a problem when snoozing a timer for the second time. I get the option to snooze again and when i click it I can also select for how long i want to snooze but then it says "This interaction failed". Did this happen to any of you as well?

#
wookie184
[python-discord/bot] New review comment on pull request #2418: Take activity into account in talentpool

Normal text channels (i.e. not threads) should always be cached, it's something we assume quite a lot throughout the bot.

I do agree that this isn't ideal, although i'm not sure what the nicest approach is. At the very least it's annoying to have my editor complain that it could be None.

Since it's something that shouldn't be None, I don't think we should just ignore if it is (e.g. early return), as this could hide some bugs.

One solution is something i'd thought of when thinki...

#
wookie184
[python-discord/bot] New review comment on pull request #2418: Take activity into account in talentpool

The check just aims to remove people who have e.g. forgotten about the server or don't use it any more, rather than being a specific "activity quota". That's the reasoning for counting any messages as some activity.

We could have it as some sort of very low activity quota, although it would require some discussion for what value to choose, and the value picked would be somewhat arbitrary.

regal archBOT
odd spireBOT
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
Robin5605
[python-discord/sir-lancebot] Pull request opened: #1223 Undeprecate bookmark

Relevant Issues

Closes #1219

Description

Undeprecated the .bookmark command, for reasons outlined in #1219. A majority of the code and logic was a straight copy paste from an older version of this repository, however some changes have had to be made to be compatible with the new version (as the rewrite to a context menu command had come with other code changes as well)

Did you:

odd spireBOT
regal archBOT
regal archBOT
regal archBOT
#
shtlrs
[python-discord/sir-lancebot] New review comment on pull request #1223: Undeprecate bookmark text command

I'm just putting this so that future reviewers are aware, but also so that you don't end up doing something that belongs to a separate issue in case you get comments about this. But currently, if DMs are closed, and you try to bookmark a message, you'll get the view that says "Receive bookmark", and if you click on it it'll tell you that you've already received it, when in fact you haven't.

Relevent issue is #1179, and should be implemented & fixed there

odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
shtlrs
[python-discord/site] New review comment on pull request #861: New Filtering System

We can probably condense this one & _create_filter_list_meta_extra_kwargs like this

def _create_meta_extra_kwargs(for_filter_list: bool) -> dict[str, dict[str, bool]]:
    """Create the extra kwargs of the for either FilterList serializer's Meta class."""
    extra_kwargs = {}
    for field in SETTINGS_FIELDS:
        field_args = {} if for_filter_list else field_args = {'required': False, 'allow_null': True}
        if field in ALLOW_BLANK_SETTINGS:
            field_args['al...
regal archBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
#
Ibrahim2750mi
[python-discord/bot] New comment on issue #2423: Why are groups still used in \`tags\.py\`

This is the reason I made the issue(if there already an issue for this, then I am sorry), I could sort them up in the sub-directories.

But first I think we need to finalise the groups we are going to have. I am thinking something along these lines.

  • python-strings
    Tags related to strings fstring, str-join etc.
  • python-discord
    Tags related to us like codeblock, ytdl, xyproblem etc.
  • discord.py
    Tags related to d.py
  • A-vs-B (can't seem to figure out a better name for t...
regal archBOT
regal archBOT
#
janine9vn
[python-discord/bot] New comment on issue #2423: Arrange tags in sub\-directories to utilise the group feature from \`tags\.py\`

I don't think we need to create groups for every single tag or for most tags. Any additional grouping we add means you need an additional term to invoke it. With your proposal, calling the pep8 tag would be !python-environment pep-8. So I want to really avoid grouping things just to group them, there should be a natural purpose to it besides "we're not using this code feature yet." The purpose behind groups was to avoid cluttering the namespace and avoid conflicts around calling tags, so th...

regal archBOT
#
Ibrahim2750mi
[python-discord/bot] New comment on issue #2423: Arrange tags in sub\-directories to utilise the group feature from \`tags\.py\`

Can't we do something like we only require the group name for triggering some specific group tags, an example would be dpy and use the other groups to list the tags in the embed more cleanly? We can check if the tag.group is an enforced(enforced meaning it would require the user to add the group name before the tag to invoke it) group if not then it can be triggered by both !group tag and !tag? For dpy we can make it an enforced group.

regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
HassanAbouelela
[python-discord/bot] New comment on pull request #2408: \[POC\] Scaffold server config via a bootstrapping script

Hey there's one thing I forgot, the current bootstrap doesn't handle off-topic channels which automatically change names every 24 hours. Can we special case OT channels during bootsrap? Generally, the names will always take the form of ot<int>-name..., and be in the Off-Topic category. If you have a preferred implementation, please proceed with it, otherwise let me know.

regal archBOT
odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
#
shtlrs
[python-discord/bot] New comment on pull request #2408: \[POC\] Scaffold server config via a bootstrapping script

Hey there's one thing I forgot, the current bootstrap doesn't handle off-topic channels which automatically change names every 24 hours. Can we special case OT channels during bootsrap? Generally, the names will always take the form of ot<int>-name..., and be in the Off-Topic category. If you have a preferred implementation, please proceed with it, otherwise let me know.

It's been handled in 4b4b65bfe5b923e9c2ac6f6cc137210fa936ccdf

odd spireBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
wookie184
[python-discord/bot] New review comment on pull request #2422: Add warning message when using \`input\` with \`eval\` command 
            # the error and the error is not manually printed by the author with a syntax error.
            if results["stdout"].rstrip().endswith("EOFError: EOF when reading a line") and results["returncode"] == 1:
                warning_message += ":warning: Note: `input` is not supported by the bot :warning:\n\n"

I'd suggest:

  • Double quotes for consistency
  • Using results["stdout"] instead of output so the warning is still displayed if the error doesn't fit ...
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/king-arthur] New branch created: Fix\-cronjob\-integrations
odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/bot-core] New branch created: Bump\-dependancies
#
ChrisLovering
[python-discord/king-arthur] branch deleted: Fix\-cronjob\-integrations
odd spireBOT
regal archBOT
#
ChrisLovering
[python-discord/bot-core] branch deleted: Bump\-dependancies
#
shtlrs
[python-discord/bot] Pull request opened: #2425 Bundle workflows into a "main" one

This adds a "main" workflow that'll be triggered upon pull_request or push to our repo's main branch, which would then "orchestrate" the other workflow that needs to be ran

Build & deploy have been fused together since they basically need each other to run.

main.yml will always lint & test, then:

  • If it's a pull request event, it'll just trigger the status embed
  • If it's a push due to a merge of a pull request, it'll lint & test, build & deploy, then creates a sentry release
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
night lilyBOT
#
Sir Lancebot

Connected!

regal archBOT
odd spireBOT
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
#
MarkKoz
[python-discord/snekbox] New comment on pull request #159: File system and Binary file sending

Is there a reason we would rather implement a file ignore system than disable Python bytecode generation? Are there eval use cases that would rely on it being generated?

Does the current ignore system support ignoring directories? The fnmatch docs state that a forward slash is not treated specially, but I'm unsure of what that means in practice. In any case, it looks more limited than gitignore so documentation is better off avoiding making such comparison.

night lilyBOT
#
Sir Lancebot

Connected!

regal archBOT
odd spireBOT
regal archBOT