dev-contrib

[still comet]

I think linking the API docs in the tag would be enough (https://discord.com/developers/docs/topics/gateway#gateway-intents)

[midnight jasper]

aight

[sharp crag]

Yeah, sorry should've mentioned that

[midnight jasper]

i recommited

[sharp crag]

Last few change requests. Once those get incorporated we can look into merging it :)

(Please make sure the code example is wrapped in a code block, not sure if GitHub caught that properly)

[midnight jasper]

code block?

[sharp crag]

Yeah:
```py
<code here>
```

(Just not sure if GitHub captured my codeblock in the review properly)

[midnight jasper]

aight reviewed and commited

oh wait

ur right

[sharp crag]

ah yeah, missing the closing ```

[midnight jasper]

ur code block

isnt completed

[sharp crag]

Yup

[midnight jasper]

it just said smth abt suggestion to batch

idk what that is

[sharp crag]

Yeah that's what was borking up the codeblock :P

[midnight jasper]

I wrote a comment

on the place u added it.

[sharp crag]

I have oneeeeee more little suggestion but that should be real easy ~

Thanks for your patience :3

[midnight jasper]

Kk

[midnight jasper]

no problemo

Ur suggestions were very good. I gotta be more attentive to things like that next time.

[sharp crag]

No worries. That's what the review process is for. Keep an eye on your notification feed if someone else also drops a review, btw

[midnight jasper]

Also I reviewed ur new comment

[midnight jasper]

gotcha

My first time doing an upstream PR

lol

[sharp crag]

Was the codeblock closed BTW? Not sure if it's just not updated on my end

[midnight jasper]

Nope

I dont see the suggestion.

Maybe write it as a new suggestion.

idk how that works

[sharp crag]

GitHub doesn't like the fact that there are two closing ```, can you edit the file and add a closing ``` from your end?

[midnight jasper]

lets see

[fallen patrol]

*zero

assuming its been pushed there's no closing ```

[sharp crag]

I meant in the review ~

[fallen patrol]

also

[midnight jasper]

How do i edit it from GH?

[fallen patrol]

ah

[sharp crag]

(Maybe that's just me being incompetent with markdown, haha)

[fallen patrol]

also Discord should be capitalised

[midnight jasper]

Where?

[fallen patrol]

- The discord gateway
+ The Discord gateway

[midnight jasper]

ah

[midnight jasper]

yo how did u do that

with the colors

[fallen patrol]

fuck it i left a comment

[sharp crag]

Navigate to the file, and hit the little "edit" button (might be hard to see)

[midnight jasper]

oh in the fork?

i thought u meant in the pr

let me try

[sharp crag]

If you edit the fork and commit it'll reflect in the PR as well

[fallen patrol]

^

[midnight jasper]

Is that how u left a review?

[sharp crag]

That's probably just GitHub automatically making the codeblock but when you run the tag on Discord, I don't think it'll auto-close

[midnight jasper]

I wanna leave a review

[sharp crag]

On your own code? :P

That's usually atypical

[midnight jasper]

i shld just edit the file?

[sharp crag]

Yup

[midnight jasper]

kk

[sharp crag]

That'll automatically reflect upstream

[midnight jasper]

Without this intent, discord.py wouldn't know which command handler to trigger so none of your commands will work.

hm

Isn't the significant thing to mention is that prefix commands rely on message content intent?

Without it, prefix commands wouldnt work.

[still comet]

I think they're called text commands

[midnight jasper]

Text commands?

Ive never heard that termnology

[still comet]

!command

[midnight jasper]

ye thats a prefix command

[sharp crag]

If you'd like to specify it's a prefix command / text command feel free:

- Without this intent, discord.py wouldn't know which command handler to trigger so none of your commands will work.
+ Without this intent, discord.py wouldn't know which prefix command handler to trigger so none of your commands will work.

[still comet]

I thought they were called text commands

[sharp crag]

I do believe they're called prefix commands yeah

[midnight jasper]

wdym by prefix command handler to trigger?

[sharp crag]

@bot.command() are "command handlers" unless there's a better name for them

[midnight jasper]

+ Without this intent, discord.py will lack the event required to dispatch prefix commands, so none of your commands would work.```

[sharp crag]

Would

Without this intent, discord.py wouldn't know which prefix command to trigger so none of your commands will work.

sound better?

[midnight jasper]

Idk i thought like registers

[midnight jasper]

how abt this. this signifies it lacks the ability to know when

[still comet]

Without this intent, discord.py wouldn't be able to detect prefix commands so they won't work.
``` this maybe?

[midnight jasper]

since the bot commands rely on the message content intent

[sharp crag]

I like this too

"detect" sounds like a more accurate word

[midnight jasper]

hm

[sharp crag]

I'm between "detect" or "recognize"

[midnight jasper]

i agree with detect

shld we specify that it lacks the event?

which is why is cant detect it

+ Without this intent, discord.py will lack the event required to detect prefix commands, so none of your commands would work.```

smth liek that

[sharp crag]

I would rather the focus be on how rather than why

Not sure if why the event isn't dispatched is all that important in this scenario. Since it's a tag we really want it to just be a "problem - solution" type deal

[midnight jasper]

would this work then?

i removed the dispatch part

focusing on the detection

unless u dont want the event

part

[sharp crag]

I'm not a fan of the "discord.py will lack the event required", as that's straying into "why" territory again

[still comet]

this might be pedantic, but "none of your commands would work" isn't necessarily true because slash commands don't care about message_content

[midnight jasper]

wouldnt that be more helpful for the people. provides more information.

[midnight jasper]

i could make it say prefix commands won't respond.

[sharp crag]

Eh I don't think most people care why, they just want to fix their commands :P

[midnight jasper]

hmm

[sharp crag]

Which is why I strongly prefer the "problem? here's the solution:" format

[midnight jasper]

cause i remember when i was tryna learn how dpy worked. any info abt how it worked was interesting. if u feel strongly then we can just have it without it. but i feel like some ppl, like me, may find interest in that sort of thing. plus, the answer to their question is still there. just a little tidbit of fun 😄

[sharp crag]

You're right that some people do but honestly most people don't and if they really wanted to know why, they can ask that as a follow up question to whoever is helping them

[midnight jasper]

hm i c

[sharp crag]

And since these are tags we're short on space so we need to cut out as much extraneous information as possible

[midnight jasper]

kk

let me resend final draft

[sharp crag]

Thanks 😁

[midnight jasper]

+ Without this intent, discord.py won't be able to detect prefix commands, so prefix commands won't respond.```

[sharp crag]

That's perfect

[midnight jasper]

aight

also, i like the idea for mentioning how u can enable them in dev portal

[sharp crag]

I do too, how are you thinking of phrasing it?

[midnight jasper]

let me write it out

+ Intents are like permissions, and privilaged intents such as message content, are enabled via discord developer portal.

this is right under

The discord gateway only dispatches events you subscribe to, which you can configure by using "intents"

So it describes intents, and tells you where to find them.

[sharp crag]

I'm thinking something like:

Do note that you also need to enable the message content intent from your [developer portal](https://discord.com/developers/applications).

[midnight jasper]

ooh good idea with the link

Do u think we shld describe what intents are tho

relating to my learning again, i didnt get what an intent was bc every tutorial or site assumed we know what it was.

i think emphasizing that its a permission could be beneficial

[still comet]

Something like this? ```
Intents are permissions that determine what events your bot receive.

[midnight jasper]

lets cut it down a bit tho, so that it can fit like an introduction to the first sentence

[still comet]

and then re-word the part about message content intent to be not redundant

[midnight jasper]

So maybe like:

[sharp crag]

We already have that as the first sentence

The discord gateway only dispatches events you subscribe to, which you can configure by using "intents"

[midnight jasper]

Privilaged intents (permissions) such as message content, are enabled via discord [developer portal](https://discord.com/developers/applications).

[midnight jasper]

true

could we sneak in that its a permission here tho. just bc it doesnt state that in the first sentence.

idk if its too nitpicky

[sharp crag]

Let's introduce it as part of the first sentence, otherwise it feels repetitive. Something like:
The discord gateway only dispatches events you subscribe to, which you can configure by using "intents," which are similar to permissions?

Oh, actually we can introduce the link there as well

The discord gateway only dispatches events you subscribe to, which you can configure by using "intents." These intents are similar to permissions and should be configured from the [developer portal](https://discord.com/developers/applications).

[still comet]

Normal intents / privileged intents are different though, I think this implies that all intents have to be configured in the dev portal

[sharp crag]

yeah i can see that

Got any ideas for differentiating them?

[midnight jasper]

+ Privilaged intents, such as message content, have to be explicitely enabled in the discord developer portal.

How bout that

[still comet]

I would add something like before they can be enabled in your code at the end

[sharp crag]

I like that.

The discord gateway only dispatches events you subscribe to, which you can configure by using "intents." 
Privileged intents, such as message content, have to be explicitly enabled from the [Discord Developer Portal](https://discord.com/developers/applications) in addition to being enabled in the code:

<code example>

[midnight jasper]

thats perfect

I misread the ending. aight ill commit that

aight i moved the code example

to above the prefix command part

to be right below that message

@sharp crag ready for u to review 😄

[sharp crag]

I am happy with this. Can we also fix the lowercased "discord" that arl suggested too?

[still comet]

would moving the last paragraph to after the first paragraph (line 7) make more sense?

[sharp crag]

Oh, and I know I said you can pick the tag filename but I was thinking something along the lines of message-content.md since that works best for fuzzy search

[midnight jasper]

yep. where is that?

[sharp crag]

Line 6

[sharp crag]

I'm impartial to that

[midnight jasper]

got it

[sharp crag]

Actually I think I prefer having it at the end since it's sort of a "why" rather than a "how," so it feels right being a "footer". Unless you have any objections

[still comet]

I feel like it should explain what message content is first before showing how to enable it

[midnight jasper]

also ye shld this be changed to Gateway Intents

someone else pointed that out

[sharp crag]

So move that last line to line 7?

The discord gateway only dispatches events you subscribe to, which you can configure by using "intents." 

The message content intent is what determines if an app will receive the actual content of newly created messages. Without this intent, discord.py won't be able to detect prefix commands, so prefix commands won't respond.

Privileged intents, such as message content, have to be explicitly enabled from the [Discord Developer Portal](https://discord.com/developers/applications) in addition to being enabled in the code:

<code example>

[still comet]

Yes, that looks good

[sharp crag]

Awesome

[midnight jasper]

+ The Discord gateway only dispatches events you subscribe to, which you can configure by using Gateway Intents. 
``` shld we change end to Gateway Intents

[midnight jasper]

just updated

[sharp crag]

Eh, probably no need

I'm happy with it as is

[midnight jasper]

someone else pointed that out and i agree it might be a bit ambiguous.

but if u dont think it rlly matters, then its all good

[sharp crag]

@still comet thoughts on that?

[still comet]

I'm fine with both

[sharp crag]

We can leave it as is then

[midnight jasper]

ok

also, period inside quotations or outside

[sharp crag]

Inside

[midnight jasper]

aight

then

ill recommit

this is like an english paper xD

[sharp crag]

Haha, the less content there is the more issues you'll find in it :P

This is great. Thanks for your work @midnight jasper and @still comet for your feedback!

We'll leave this up for anyone else who wants to review or a core dev to approve and merge

[midnight jasper]

This was lowkey fun. If u need smth else for me to do, ping me.

[sharp crag]

Feel free to also do the remaining checkbox in https://github.com/python-discord/site/issues/695 if you're up for it

[still comet]

Can I do the @bot.event/on_message one?

[sharp crag]

Sure, go ahead!

[midnight jasper]

Sure.

[midnight jasper]

I can help if u need to run some ideas by us.

[still comet]

sure I'll ping you (if you don't mind) when I get started later

[sharp crag]

Mind letting @still comet do this one?

[midnight jasper]

Ye ofc. I didn’t realize the other checkbox was that one he was referencing.

[midnight jasper]

Ye np!

[sharp crag]

Could you also ping me here on Discord when you request a review? Sometimes I miss GitHub notifications, but it's harder to miss discord pings :P

[still comet]

sure

[sharp crag]

Awesome, thank you

[sharp crag]

listen here you little-

[still comet]

@sharp crag @midnight jasper https://github.com/python-discord/bot/pull/2567

Is the bot.process_commands() solution worth keeping? I feel like it's just taking up space for no good reason

I also considered adding a link to the FAQ on the docs regarding this (https://discordpy.readthedocs.io/en/latest/faq.html#why-does-on-message-make-my-commands-stop-working) but it doesn't go into much depth anyway so I decided against it

[midnight jasper]

How do I write a review with editing part of it like Robin did.

[rapid swallow]

like this?

it's called a suggestion

[midnight jasper]

yes

[midnight jasper]

@sharp crag ?

[sharp crag]

Was that the one from yesterday?

[midnight jasper]

Ye

[sharp crag]

Ah sorry, I must've forgotten to explicitly approve

I'll do that when I get home today and check our code golf's PR too

[midnight jasper]

Kk

ima write a review for his

@still comet wdym

You can also tell discord.py to process commands as usual after you're done processing messages

U don't process the messages, no? U just have to process the registers commands to invoke them using the process commands.

I submitted a review, but didn't review that part, as I'm waiting for confirmation.

[midnight jasper]

@vocal wolf https://github.com/python-discord/bot/pull/2056 do u need someone to take this up

[still comet]

Yeah, I'll change that if we decide to keep that section

[midnight jasper]

I think we should keep the section.

People should know why. It’s not helpful for ppl if they don’t know why.

[still comet]

I feel like it wouldn't be relevant for most use cases of the tag though

The helper could explain it if the helpee brings it up, it would just unnecessarily take up space most of the time

[still comet]

When can you do that? I have to go to sleep soon

[sharp crag]

It'll probably be a few hours, but GitHub stuff is asynchronous so it's all good

[still comet]

Alright

[still comet]

Any thoughts on this?

[slim widget]

@clever wraith We don't allow recruitment in this server.

[midnight jasper]

@timid sentinel Alright I commited ur review and changed the name. I also added some aliases.

😄

[sudden pecan]

hello

i love htis

[slim widget]

Hey. What's our process for selecting PR reviewers. Do I just volunteer someone? 😄

[midnight jasper]

Its an official position?

[thorny obsidian]

Usually you request from someone who has a vested interest in that code area. Like if it were messing with events I would want a review requested from me. But otherwise I usually don't request anyone specific or have Xith do some poking

[vocal wolf]

poke me and I will poke many more

[static canyon]

poke
bot#2089, bot-core#137 and bot-core#141 are all PRs I did last year that still need reviews

There's also my more recent (~3-4weeks) bot#2520

[dusky shore]

GitHub

[bot] #2089 Feat: add reminder snoozing & show new expiry / mention count when editing reminder
[bot-core] #137 Add check_rerun_command util to commands.py
[bot-core] #141 Migrate all message utils from bot and sir-lancebot.
[bot] #2520 Handle off-topic channel names that are invalid for servers in Server Discovery

[static canyon]

I think #141 was semi-blocked by not having a decision on how to deal with constants config in bot-core, but in all honestly I don't see why that should stop the PR. We should be able to just merge as-is and if needed adjust later (it's a small part of the PR anyway)

[vocal wolf]

last year

oh no

[static canyon]

Indeed. They seemed to just fall on deaf-ears so to speak. Have asked for reviews a couple times here but...

[vocal wolf]

I have poked people

[vocal wolf]

Let's see. @clever wraith, do you have plans for this PR? Thanks!

https://github.com/python-discord/bot/pull/2056

[static canyon]

Thanks Xith :)

[static canyon]

Ah, actually, there's also bot#2461 and bot#2486 which I asked to be assigned to whilst you're here. Not sure if I wasn't assigned simply because it wasn't seen or whether there's some other factor

[dusky shore]

GitHub

[bot] #2461 Detect codeblock escape for file contents/file paths in eval
[bot] #2486 Forbidden: 403 Forbidden (error code: 90001): Reaction blocked

[vocal wolf]

@sharp crag Greetings. To add onto the opinions generated within the PR below, I believe it should probably be closed. The !eval command works pretty well on its own.

https://github.com/python-discord/bot/pull/2269

[sharp crag]

I agree 👍
Has been closed.

[vocal wolf]

[outer oasis]

Oh are we looking at issues?
Can I beg and whine and cry for bot-core#30 ?

[dusky shore]

GitHub

[bot-core] #30 snekbox wrapper

[vocal wolf]

just more discussion?

[sharp crag]

he has ulterior motives

[outer oasis]

I want it implemented so I can steal it

[sharp crag]

told you

[vocal wolf]

lol

first link in issue: #dev-contrib message

[outer oasis]

Ye
Is assigned, but no do

[vocal wolf]

@clever wraith when do thing

[outer oasis]

BTW, while I have you all here

Is this something you've seen before?

[vocal wolf]

is this sentry?

[sharp crag]

yes

[outer oasis]

I have absolutely no idea where to even start

[outer oasis]

Yeah, with !int e ripped out of @stable mountain and hammered into my own bot

[vocal wolf]

hmmmm

[outer oasis]

Oh I cut off the header
My bad
Yeah it's my own repo

[vocal wolf]

Just got to them, I dub thee assigned.

[static canyon]

Thanks :)

[vocal wolf]

@sharp crag thing bot#2402

[dusky shore]

GitHub

[bot] #2402 reply to user when using !eval/!timeit command instead of pinging

[outer oasis]

Err, wait. It's actually stolen from @dusky shore
@stable mountain's is much shorter
... why so different?
Maybe I'll try @stable mountain's version instead

But yeah, if the issue goes through, then I can consolidate this and more under the same roof

[sharp crag]

Will do 👍

[vocal wolf]

thank

[fading galleon]

@vocal wolf I have previously mentioned that I am abandoning bot#2446

[dusky shore]

GitHub

[bot] #2446 Migrate !source to /source (app_commands)

[fading galleon]

I am kind of extremely busy now

[vocal wolf]

Ah, I see. Next time, please make this clear to a core developer or comment in the PR. Would you like me to put it up for grabs so someone can take over the PR?

[fading galleon]

I had told @clever wraith in this channel earlier and iirc they pinged you.

Yes mark it up for grabs.

[vocal wolf]

Ah, my bad. Might have missed it.

[static canyon]

Does anyone have any ideas why the cogs aren't loading when I run @dusky shore locally? No errors or anything, it just doesn't try to load them

Edit: seems to work now

 ⠋ sir-lancebot The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested  0.0s
Attaching to sir-lancebot, sir-lancebot-redis-1
sir-lancebot-redis-1  | 1:C 02 May 2023 02:41:44.151 # oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
sir-lancebot-redis-1  | 1:C 02 May 2023 02:41:44.151 # Redis version=7.0.11, bits=64, commit=00000000, modified=0, pid=1, just started
sir-lancebot-redis-1  | 1:C 02 May 2023 02:41:44.151 # Warning: no config file specified, using the default config. In order to specify a config file use redis-server /path/to/redis.conf
sir-lancebot-redis-1  | 1:M 02 May 2023 02:41:44.152 * monotonic clock: POSIX clock_gettime
sir-lancebot-redis-1  | 1:M 02 May 2023 02:41:44.153 * Running mode=standalone, port=6379.
sir-lancebot-redis-1  | 1:M 02 May 2023 02:41:44.153 # Server initialized
sir-lancebot-redis-1  | 1:M 02 May 2023 02:41:44.155 * Ready to accept connections
sir-lancebot          | 2023-05-02 02:41:47 | root | INFO | Logging initialization complete
sir-lancebot          | 2023-05-02 02:41:47 | pydis_core.utils._monkey_patches | DEBUG | Patching send_typing, which should fix things breaking when Discord disables typing events. Stay safe!
sir-lancebot          | 2023-05-02 02:41:47 | asyncio | DEBUG | Using selector: EpollSelector```

[static canyon]

VSC is also giving me loads of pylance and mypy errors

[midnight jasper]

What is required to be changed to become /source I can take that one up before quwerty's if you need.

[vocal wolf]

https://github.com/python-discord/bot/pull/2446#issuecomment-1499020850 this needs to be resolved

[midnight jasper]

I can try and figure that one out.

[vocal wolf]

I can assign you to that PR, if you'd like

[midnight jasper]

ye sure!

[vocal wolf]

Comment on the PR saying that you'd like to take it

github requires a person to participate in the PR or be in the org in order to be assigned

[midnight jasper]

Okay

[vocal wolf]

at least, that's how I remember it working

[midnight jasper]

Alright, commented!

[vocal wolf]

it is done

[midnight jasper]

What does the source command do?

Based on the problem displayed, there seems to be an issue with the autocomplete.

[still comet]

give you the source for a cog/command/tag

[vocal wolf]

within github, specifically

[midnight jasper]

Oh thats interesting.

[vocal wolf]

it gives you the link

!source help

[static canyon]

!src src

[stable mountain]

Command: source

Display information and a GitHub link to the source code of a command, tag, or cog.

Source Code

Go to GitHub

bot/exts/info/source.py:21

Help Command

An interactive instance for the bot help command.

Source Code

Go to GitHub

bot/exts/info/help.py:167

[midnight jasper]

thats actually very cool

So right now the autocomplete has to be fixed for that?

Seems doable 😄

[vocal wolf]

The PR is a migration from a regular command to a slash command

switching the API in the process, I believe

I haven't worked with the bots in a while

[static canyon]

Isn't the src command logic something that should be on botcore? Given that it's presumably the same on both bots

[vocal wolf]

hmmmmmm

[midnight jasper]

Mhm. I am not sure how much is done. Is it just fixing the autocomplete or do I have to provide the functionality of actually retrieving the link from the name of the file provided.

[vocal wolf]

sounds like an issue to be put in bot core

[midnight jasper]

Also, @sharp crag any update on the tags?

[vocal wolf]

@static canyon nevermind it already exists

https://github.com/python-discord/bot-core/issues/46

[static canyon]

Ah, cool

[vocal wolf]

@spare plaza it's been more than 2 weeks you can pick this up bot-core#46

[dusky shore]

GitHub

[bot-core] #46 Moving Converters and the Source Cog into botcore

[midnight jasper]

sorry I forgot ping @vocal wolf 😅

[static canyon]

The code for giving the github link is already done

So I guess it's just the auto-complete

[midnight jasper]

Alrighty.

[static canyon]

/source item:{shows_suggestions_with_auto_complete}

[sharp crag]

Working on that right now.

[midnight jasper]

Gotcha.

[sharp crag]

Left a review. Small issue with formatting.

[midnight jasper]

Ah yep sorry, that was a dumb mistake lmao.

Alright @sharp crag recommited fix.

I think we are waiting on Wookie right?

[sharp crag]

I believe so

[midnight jasper]

@vocal wolf what does this mean?

I haven't gone this in depth to GH PRs so I'm a bit ignorant in this topic.

Ill submit a review tho if needed.

[vocal wolf]

You can go into the "files changed" tab under the title of the PR, and then you can click on specific lines or click-drag over multiple lines to review a chunk of code

[sharp crag]

It means a user (in this case, Xith) wants you to review the PR you're currently looking at. See above message ^ for how

[vocal wolf]

Once you click on line(s) it'll give you 3 options, which are cancel, add single comment, or start a review

[midnight jasper]

Oh ye, I submitted two reviews before on this repo. Just curious, if there is anything specific you want me to review.

[vocal wolf]

just the changes made within the PR

wait holup you're the author of the PR

since I assigned you

[midnight jasper]

The PR im referencing is the on_message_event tag, where it says u required a review.

[vocal wolf]

ah

yeah I re-requested a review or something I believe

[midnight jasper]

Oh, I see.

[vocal wolf]

I went through a bunch of PRs, forgot the context lol

[midnight jasper]

lmao no problem 🙂

[vocal wolf]

ya so just review stuff that was changed

[midnight jasper]

@still comet I left a review for you to check out.

[sharp crag]

As have I

[still comet]

@sharp crag @midnight jasper thoughts on https://github.com/python-discord/bot/pull/2567#discussion_r1182057903 ?

I feel like it's still a little redundant with the bot.process_commands() section

[clever wraith]

I had completely forgotten about this...

@outer oasis you can have this if you want

Want me to assign you?

[spare plaza]

nah

[vocal wolf]

aight

[midnight jasper]

Which part?

[still comet]

However, this method isn't recommended as it does not allow you to add multiple on_message handlers."

I feel like it could be combined with the other one somehow

[midnight jasper]

Oh yes, this part is a bit redundant because up here it says

Listeners get added alongside the default on_message event, rather than overriding it, so prefix commands can still be invoked as usual:

Not sure if u rlly want that part to be incorporated. That is just ur personal preference.

[fallen patrol]

autolinking format is broken disnakedev/disnake#997

yw

[dusky shore]

GitHub

[disnake] #997 feat(permissions): add use_external_sounds

[static canyon]

I don't think so?

It just took a while to respond from the looks of it

[fallen patrol]

look at the embed

[static canyon]

?

It's showing disnake#997, which is what you asked for

[fallen patrol]

not the intended UI

[static canyon]

Oh, that's your end

It's fine for me

[midnight jasper]

Sir Lancebot uses disnake?

[fallen patrol]

update your app

[midnight jasper]

Another d api wrapper?

[static canyon]

It is updated (there's no "update available" button)

[fallen patrol]

what app version?

app version is in app settings at the bottom

[static canyon]

Besides, it's probably just because of the new markdown changes

[fallen patrol]

what app version?

[static canyon]

Stable 194296 (68c6883) Host 0.0.275 OS X 10.15.7 (22.4.0). And reloading Discord doesn't change anything, so I'm on the latest for my client/os/whatever

[fallen patrol]

canary is 194529

[static canyon]

It looks like your client just doesn't handle the nested [] for some reason, but mine does

[static canyon]

Also ^

[fallen patrol]

anyways that's the report, when it is either patched by discord in a few days or comes to mobile you can choose what you want to do then

[atomic ivy]

it looks weird on the new mobile UI as well

[fallen patrol]

dw, I'm acutely aware of them

[timid sentinel]

what's the cause? nested []?

[fallen patrol]

honestly

[clever wraith]

yes

[fallen patrol]

who fucking knows

[midnight jasper]

Lancebot uses that wrapper?

[static canyon]

Either that or discord disabling hyperlinks entirely because of the security issue they found with new markdown (or codeblock)

[fallen patrol]

disnakedev/disnake#996 they aren't disabled

[dusky shore]

GitHub

[disnake] #996 fix(typing): update misc payload types

[fallen patrol]

its just different behaviour

i think its because of the codeblock within the title

lemme try and remember the number

[timid sentinel]

ah. well, probably best to wait for a bit and see if discord fix it.

[fallen patrol]

discord/discord-api-docs#6088 ?

[dusky shore]

GitHub

[discord-api-docs] #6068 Banning with delete_message_seconds does not delete messages

[fallen patrol]

damn it one minute

oof the embeds don't automatically update

forgot that wasn't a feature

discord/discord-api-docs#6088

[dusky shore]

GitHub

[discord-api-docs] #6088 Link Markdown Fails

[clever wraith]

No, we use discordpy

[fallen patrol]

but y'know, its your call whether to implement these or not

[fallen patrol]

they were going to switch to disnake--and did for a short time--but opted to switch back when discord.py was unarchived after the 9-10 month hiatus

[midnight jasper]

Hm thats what I thought, I was confused about the disnake embed.

[midnight jasper]

Ah I see. Wait, did u make it?

[fallen patrol]

ah, because i work on disnake so i used that as an example as that issue is where i noticed it

[outer oasis]

Yeah, go ahead
I'll need some help, but I want to see it completed, so I'll get it moving

[eternal owl]

hey @static canyon , looks like there are conflicts in the the off-topic reroll PR

[static canyon]

Will take a look, thanks

[static canyon]

It's just because the '' were replaced with "". I'm committing the resolution now 👍

[static canyon]

Done, thanks again for letting me know 👍

[eternal owl]

I will review the PR tmw, thanks for resolving the conflicts

[static canyon]

Sounds good, thanks 👍

[clever wraith]

You don't need to, i've assigned you.

[outer oasis]

Well I didn’t want to actually WORK
I just wanted it done for me!!

I’ll take a look

[clever wraith]

What do you think you'll need help with?

(because that's what you said)

[outer oasis]

I’m just brand new to bot dev entirely, and I’m still struggling to understand what’s going on

Also, for example, the internal eval cog is a single file on Python and an entire package on Lance.
Which case do I wrap?
Are they different for a reason?

It’s mostly just
🛫
🐕‍🦺
Still gotta get my head around it

[cold island]

Snekbox eval and internal eval are unrelated

[outer oasis]

We're wrapping all of the above

They were talking about latex too

Just a generic wrapper that takes input, sends it away for processing, and then returns the results in an embed with a trashcan

[midnight jasper]

I can help you out. I love making discord bots!

[outer oasis]

I'm at work right now, but here's the summary:
!int e: https://github.com/python-discord/bot/blob/main/bot/exts/utils/internal.py
!eval: https://github.com/python-discord/bot/tree/main/bot/exts/utils/snekbox
.latex: https://github.com/python-discord/sir-lancebot/blob/main/bot/exts/fun/latex.py
bot-core#30 noticed that these all take input, send it to a URL, [well, int-e doesn't] and then return an embed with the response. And has the concurrency limit, uploads to paste.pythondiscord.com if it's too long, etc, etc
The plan is to create a class in bot-core that provides a interface that abstracts away all that I/O, so we can just implement it on all the services, which cuts down on duplication and increases consistency.

[dusky shore]

GitHub

[bot-core] #30 snekbox wrapper

[timid sentinel]

Tbh I think snekbox and internal eval are different enough that I'm not sure there's much to be gained from trying to combine them.

[outer oasis]

I'll admit I've started second guessing internal eval a bit, since it doesn't actually go out to a URL like the others

[fallen patrol]

agreed.

don't even have to look at the source

if you combine them make sure you didn't leave a security hole where an eval command can be redirected to internally 🤡

[outer oasis]

well don't just give away all my plans!!

[midnight jasper]

@outer oasis wdym eyes

[midnight jasper]

Which I/O?

Do u wanna abstract.

[midnight jasper]

I came up with a good idea for one of the fun bots. People are timing how long it will take for someone to mention Chat GPT, so could I code that into one of the fun bots where every time someone says Chat GPT the timer resets. Then there is a command to check the timer.

[outer oasis]

don't encourage them

[gritty wind]

Just always send a 0

[tawdry vapor]

They have some things in common, and could have more feature parity if we wanted. For example, reevaluating an edited code block, output truncation, uploading to paste service,etc.

It may not make sense or work out to make some common base code for them, but we should at least look into that possibility

[outer oasis]

Do you have an idea of what that would look like?

[tawdry vapor]

I think an abstract base class was mentioned but no fleshed out ideas. Might just end up being a few utility functions in a new module.

Not trying to be prescriptive on the exact implementation

[outer oasis]

I'm new at this, so some prescription may be necessary.

I wasn't sure how to make a wrapper for evaling though an API , then have it magically not use an API for int-e.
But actually, without an actual class, and just functions, it can be a lot more versatile.

Also -- I noticed that https://github.com/python-discord/bot-core/tree/main/pydis_core/exts is completely empty.
Could I just move the entire int-e cog into bot-core?
I'm curious about this one in general -- why is it there just to be empty?

[tawdry vapor]

Could have a base class with an abstract method and the concrete implementations do whatever is needed to get the eval result

That's an idea off the top of my head. There may be a better way

If you're unsure, you can come up with a design and we can review it before you actually go ahead and work on the refactoring

@vale ibex may know why that dir is there but empty

[midnight jasper]

Ur tryna improve eval?

[outer oasis]

!eval, and .latex and others, all take the command, send it to an API for processing, then send the API's response in a message
We're trying to make a wrapper for that where we can just drop in the URL, and it handles everything for us

[midnight jasper]

Oh I see. Why use an abstract method?

Have a base class that is the arbitruary wrapper, and derive a subclass per api you are dealing with.

[midnight jasper]

Whats int-e

I dont see why you would have an abc if there are methods that you want shared.

[outer oasis]

Well the idea was to not have to make a new class every time

[outer oasis]

!help internal eval

[stable mountain]

Command Help

!internal eval <code>
Can also use: internal e

You cannot run this command.

Run eval in a REPL-like format.

[midnight jasper]

For every API?

Each api would require differnet headers. So unless u passed it into the constructor, u would need something different.

[outer oasis]

Yeah
There's a only a couple things you need
A URL, some headers for auth, etc, etc
Just create an instance with those things, and you're done

Yep, exactly

[midnight jasper]

Why have an abstract base class then?

The methods should be static, so you wouldn't even need a constructor.

[outer oasis]

Just suggesting ideas

... static?

[midnight jasper]

Hm.

[midnight jasper]

Yes. Why instantiate a class every time you want to make a request to an api.

[outer oasis]

... you misunderstand

[midnight jasper]

If the only data u send is endpoint, headers, etc. why not have an enum or stringenum correspond to a certain endpoint, that way you can pass in the enum object for whatever api u want to use. Then pass in the headers.

[outer oasis]

You would instantiate the class when the bot starts up, and you would just continue interacting with it
You wouldn't reinstantiate it every request

[midnight jasper]

o

[outer oasis]

Hell, you need it there to make the request

[midnight jasper]

Hm. So why not have a method that takes in an enum and then some header object which is a dataclass.

[outer oasis]

... where would the enum come from?

[midnight jasper]

Thats an enum that we make, where each member corresponds to an endpoint.

[outer oasis]

... each instance would only have one endpoint

[midnight jasper]

...

I thought u said we use the instance for everything...

And there are multiple endpoints... no?

[gritty wind]

Could do this

# abstract
class Base:
  # abstract
  def eval(args) -> str:
    ...

  def send_result():
    # concrete
    result = eval()
    truncate, upload, etc

class Snekbox(Base):
  def eval(args) -> str:
    # concrete

[midnight jasper]

Why override it if there is only going to be one instance?

[gritty wind]

One instance of what

[midnight jasper]

The object for the apis.

[outer oasis]

One instance per API*

from pydis_core import EvalWrapper
from discord import command


class Snekiboi:
    def __init__(self):
        self.client = EvalWrapper(url="https://snekbox.com")

    @command
    def eval(self, text):
        """do the needful"""
        self.client.run(text)


class Latex:
    def __init__(self):
        self.client = EvalWrapper(url="https://latex.com")

    @command
    def latex(self, text):
        """do the needful"""
        self.client.run(text)

[gritty wind]

I don't see what that has to do with the structure of the class at all

[outer oasis]

Oooh this is clean

[gritty wind]

It's overriden because eval for some of these is a lot more complex than a simple "send to API and bob's your uncle"

Case in point: int e

The reason the botcore exts directory is empty is because we never reached a consensus on how to handle constants reasonably within bot-core. Almost any meaningful non-util ext requires some level of constants

[midnight jasper]

Oh. Ye, then have an abstract base class.

[midnight jasper]

Why have it in the base class tho if its going to be overriden per subclass?

[gritty wind]

Because the base class calls it

The base-class is there to define the interface, it's an empty body

[midnight jasper]

Oh?

[gritty wind]

(for the eval function only)

[midnight jasper]

Oh the base class is used by eval.

[gritty wind]

I mean this is generally how OOP looks like in a lot of languages

Define some method -> override it in a child

[midnight jasper]

I've seen that but I've never understood why if ur going to override it in the subclass.

But the base class can call the overriden method? I didn't think that was possible.

[gritty wind]

Because the parent and child need to agree on a standard for what everything is going to look like

[midnight jasper]

Oh. So there isn't any benefit for declaring it in the base.

[gritty wind]

If it helps, you can imagine python translating:

class Snekbox(Base):
  def eval(args) -> str:
    # concrete

to

class Snekbox:
  def send_result():
    # concrete
    result = eval()
    truncate, upload, etc

  def eval(args) -> str:
    # concrete

Under the hood of course

[gritty wind]

There is

If we didn't agree on what this method looked like beforehand, the concrete method send_result wouldn't be able to call eval, because it doesn't know what arguments the function takes, it doesn't know what it returns, it doesn't know if it even exists

[midnight jasper]

Why wouldn't it know that information if it's defined in that subclass that send_result is in.

Unless ur saying send_result is in the base class.

[gritty wind]

It is in the base class

[gritty wind]

^

[midnight jasper]

Ooh. That makes sense now.

[gritty wind]

It's repeated code that we want to abstract

[midnight jasper]

Sorry for my confusion, I haven't written with this structure before 😅

So u want the eval to change per subclass, but the send_result is the same?

Ye that is a pretty good structure.

@outer oasis Though, are you going to pass the endpoint into the constructor? I feel like maybe having a class that contains all the endpoints could be beneficial.

[outer oasis]

Why?

[gritty wind]

I believe this paradigm is referred to as prototyping or prototypal inheritance, though if you want to learn more I suggest starting with a more fundemental OOP resource to not get thrown into the deepend

[midnight jasper]

I feel its much neater and more readable if you had some constant member whose value is the endpoint.

So if u were instantiating the class u could do ```py
from .endpoints import Endpoints

abc_obj = ABC(Endpoints.eval) # Endpoint arg```

[gritty wind]

In practice this just looks like:

from bot.constants import Eval_API
do_something(Eval_API)

Defining a large list of constants at the bot-core level doesn't make much sense if you consider where the users of the API live

Latex subclass would only live on sir-lance

And if sir-lance wants to add a user, it doesn't want to PR all the way up to botcore

The only shared one I can think of is int e, but that's already a very special case of this API (not convinced it should be merged with this at all, but we'll see)

[midnight jasper]

Hm thats fair. Though, maybe with all of the endpoints in the bot, they should all be stored somewhere and abstracted over to be more readable, because just including the link there isn't as clean and a bit more hard coded. What if there was a config file which has all of the names being EVAL_API and SOMEOTHER_API and each of the values is a link that can be configured. No matter the method, placing the EPs somewhere where they can be changed easily is a good choice IMO, instead of having to hunt them down in each file.

[timid sentinel]

The latex command code being very small suggests to me that the minimum amount needed is pretty small, so I think it's probably worth considering what extra features we want where. For example, I'm not sure we'd want rerunning code for internal eval, since it's not necessarily stateless, so it's potentially useful to see what commands have been run already.

[gritty wind]

That already exists, it's the bot.constants file

[midnight jasper]

Oh.

[gritty wind]

https://github.com/python-discord/bot/blob/main/bot/constants.py#L480

[stable mountain]

bot/constants.py line 480

snekbox_eval_api = "http://snekbox-310.default.svc.cluster.local/eval"```

[midnight jasper]

Oh. What did you mean up at the top here?

Defining a large list of constants at the bot-core level doesn't make much sense if you consider where the users of the API live

[gritty wind]

Consider something like the latex API, a feature that is only important for sir-lancebot. The subclass class Latex(Base) would be written on sir-lancebot not bot-core. The constants used for this should then also live on sir-lancebot, not bot-core

[midnight jasper]

Oh. U dont want to have a place that holds the constants for lancebot?

[gritty wind]

I generally agree. IMO porting over a few of the other eval-like features is already a net positive (though doesn't latex produce JPGs where this isn't relevant?). Excluding one cog isn't the end of the world

[gritty wind]

I do, and it exists, but it's on lancebot

Not bot-core

[midnight jasper]

O. Then, what was the discrepancy.

That you didn't want it on bot-core?

[gritty wind]

Yeah, defining a large list of constants at the bot-core level doesn't make much sense

[midnight jasper]

Oh ye I agree. I was suggesting that the endpoints all be in some place like constants per bot.

Cause bot-core wouldn't need any endpoints I bet.

@timid sentinel We are ready for another review if u are ready as well.

On message content intent tag.

[timid sentinel]

I'm on my phone at the moment. I can probably take a look tomorrow. Last I looked it seemed pretty good, the mix of _ and - in tag name/alias made me think we should probably normalise that for tag invocations so either work and you don't need to remember which to use, but that's not really related to the PR so I might open a new issue at some point.

[midnight jasper]

https://github.com/python-discord/bot/pull/2446 Wait does this autocomplete have to be based on the cogs within each file?

[clever wraith]

It should be for cogs / commands and tags

We almost always have them separate in their own files

Except for commands

Since they're contained within a group / cog

[midnight jasper]

Are all the files with cogs stored in a folder?

[static canyon]

Fwiw bot-core#137 is somewhat relevant to this convo (it's a PR that adds the re-run logic to bot-core; just needs approval)
@outer oasis (think you're the one that will be implementing(?))

[dusky shore]

GitHub

[bot-core] #137 Add check_rerun_command util to commands.py

[static canyon]

So that part wouldn't be needed in your implementation

[clever wraith]

All cogs are within the ext package

Which represents the bot extensions

[outer oasis]

Yeah, I told the whale to hurry up and do the needful, and I ended up assigned to do it myself
Time to start learning

[outer oasis]

Wait why not?

Isn't that a huge part of eval?

[clever wraith]

And each extension is usually within it's own package

Like the filtering x moderation w etc

[outer oasis]

Oh you're saying it's already done, so I won't need to do it myself?
That's helpful
Can we force this here whale to approve it for us?

[static canyon]

Yeah, exactly.

@clever wraith review pls? 👉 👈

[static canyon]

^

[outer oasis]

(Has conflicts now )

[static canyon]

Does it? I don't see them (but maybe just because I'm not logged in)

Altho not surprising given I did this over 7 months ago

[clever wraith]

You don't need to be logged in to see that AFAIK

[static canyon]

The conflicts is probably just the changelog

[static canyon]

Eh, f

[clever wraith]

Honestly, I'm not entirely sure of why we would need this entire re-run business logic

[static canyon]

I'm out atm but guess I can look tonight (will leave the changelog one though)

[clever wraith]

I seem to have lost the entire context

[static canyon]

Well, the context for my PR on bot-core is that we want the re-run logic @stable mountain has for !eval on @dusky shore for .latex

[clever wraith]

It's also super late here, so it's probably best to not have this discussion now

What's wrong with the current re-run logic we have for python?

I have a blurry souvenir where it's only related to snekbox 3.10

[static canyon]

It's mostly fine, we're just moving to bot-core since now both bots need the logic

There is an issue with the current logic where re-running doesn't account for command arguments correctly, but that's explained & fixed by my PR

[stable mountain]

Sorry, an unexpected error occurred. Please let us know!

ClientResponseError: 400, message='Bad Request', url=URL('http://snekbox-310.default.svc.cluster.local/eval')

[clever wraith]

In that case tizzy, can you please make a well-defined issue for the change?
It's always better to elaborate on the problems we currently have so that everyone can have clear context, instead of having us ask you the same question over and over again

Also, I feel that we'll soon be deprecating snekbox 3. 11

. 10*

[static canyon]

It's all there in the PR

The github issue is on lancebot (altho that doesn't mention the bug in the rerun logic) under "add rerun to .latex" or something

[static canyon]

Yeah, it's not specific to that so still applies

[clever wraith]

I know that you've mentioned details in the pull request, but we always push towards having well defined issues where we can argue / discuss the problems at hand.

[midnight jasper]

Gotcha

[static canyon]

It was all discussed fwiw

[midnight jasper]

Extentsions meaning… abritruary commands?

[static canyon]

Remember, this is a PR that was left unreviewed for 7 months

[clever wraith]

No, extensions here follow the open closed principle so to say

[static canyon]

Anyway, I gtg now (going out for dinner). Can answer further questions when I get back (couple hours)

[clever wraith]

I will try to have a better read tomorrow

Thank you for all your efforts so far!

[midnight jasper]

So things you add are extentsions?

Not sure if thats the right interpretation.

[clever wraith]

Extensions, are meant to add extended features for the bot.

We're contextualizing them

[midnight jasper]

What are considered base-unextended features?

[clever wraith]

The one's you'll find in bot core mostly

Like loading extensions

Loading commands

Etc.

They're core components That the bot needs to function

[midnight jasper]

Gotcha.

[clever wraith]

To have an MVP

Modularization is ensured via these extensions

We're isolating different components, that added extra layers / features to our bot'

That's are very nice to have, but not critical for our bot to be functional.

Functional from the most basic perspective.

[midnight jasper]

Ye. Sometimes it’s a bit confusing in where certain things are 😅 but I see how the total layout makes sense.

[clever wraith]

You're always welcome to ask questions here regarding any ambiguities you might have

And we'll do our best to explain / unravel the '' mysteries ''

[midnight jasper]

Lmao I appreciate it 😄

[clever wraith]

I just saw this, 🤨

[vale ibex]

Scale and I added it way back when starting the project since we were going to add base cogs, but never did

[still comet]

I've just noticed that Lancebot's help command doesn't use HelpCommand, that probably needs fixing, right?

Or would it be better to move it to bot-core instead?

[static canyon]

Fyi Mr. Triage (@vocal wolf) there's bot#1849 which seems to have gotten lost to time. If it's something that's still wanted (CC @mellow hare) then I'd be happy to implement in the upcoming weeks/months should no one else take it up.

[dusky shore]

GitHub

[bot] #1849 Create new voice-ban command and change the current voice-ban's name to voice-mute

[static canyon]

👋 Did you manage to get a chance to take a look at bot#2520?

[dusky shore]

GitHub

[bot] #2520 Handle off-topic channel names that are invalid for servers in Server Discovery

[static canyon]

@sharp crag is bot#1223 ready for review again? Not sure since it's still tagged as waiting for author

[dusky shore]

GitHub

[bot] #1223 Adjust When the Help Cooldown Role Expires

[static canyon]

Oop

• sir-lancebot#1223

[dusky shore]

GitHub

[sir-lancebot] #1223 Undeprecate bookmark text command

[static canyon]

Just took a look and although the website says there's conflicts in botcore/utils/commands.py, I can't see where locally, and can't view on website so

Tried looking manually for potential conflicts as well and didn't see anything, so have no clue what that's about

The docs/changelog.rst conflict can be ignored until the PR needs to be merged, since if anything else gets merged before this PR then I'll have to change it again.

So @clever wraith bot-core#137 should be ready for review. Not sure if you still had any questions, but if you do just ask :)

[dusky shore]

GitHub

[bot-core] #137 Add check_rerun_command util to commands.py

[vale ibex]

the module was renamed to pydis_core

so you'll need to rebase/merge main

[outer oasis]

I think it's the renaming

hecku I was still thinking!!
Now it just looks like I copied you...

[outer oasis]

Why can't I just "update branch"?
Is it because there's also conflicts in another file?

[vale ibex]

[outer oasis]

Fix it Chris

[atomic ivy]

GitHub needs to get better

[outer oasis]

Go get a job at GitHub and fix it for us

[vale ibex]

https://github.com/about/careers !

[atomic ivy]

fine, I'll do it myself

[outer oasis]

Nothing open in my area or my field

[static canyon]

Hmm, okay. That means I have to fix the changelog conflict too, right?

Also, we don't have an else: in the handle_check_failure function, so does that mean we're actually (potentially) suppressing some errors?

https://github.com/python-discord/bot/blob/main/bot/exts/backend/error_handler.py#L317-L324

[stable mountain]

bot/exts/backend/error_handler.py lines 317 to 324

if isinstance(e, bot_missing_errors):
    ctx.bot.stats.incr("errors.bot_permission_error")
    await ctx.send(
        "Sorry, it looks like I don't have the permissions or roles I need to do that."
    )
elif isinstance(e, ContextCheckFailure | errors.NoPrivateMessage):
    ctx.bot.stats.incr("errors.wrong_channel_or_dm_error")
    await ctx.send(e)```

[vale ibex]

sure

[outer oasis]

Git question: is it possible to partially fix conflicts?
To exit the fixing [without --quittiing] without fixing everything*

[vale ibex]

the docstring states the errors that are supported by that function, so we shouldn't be sending any errors other than those to it from the actual place doing the handling

[vale ibex]

probably, never looked into it

[outer oasis]

Damn
Tizzy got to work

[static canyon]

Right, but what about the errors that are CheckFailure but not one in the docstring? Don't they just get supressed?

[static canyon]

Lmao

You should've seen some of my other ones 👀

[vale ibex]

probably, yea

[static canyon]

Eventually I'll have to do bot#2089, and that's >7 months on the bot repository. Oh, and the filter rewrite was part of that period

[dusky shore]

GitHub

[bot] #2089 Feat: add reminder snoozing & show new expiry / mention count when editing reminder

[static canyon]

Isn't that bad?

[atomic ivy]

do you have an example?

[vale ibex]

only if we use those types of checks

[static canyon]

Seems bad design to have a whole though. Some day someone may use that check, and then wonder why the error handling doesn't work

I feel there should be something

[vale ibex]

it might also be the case that we're not handling those cases, as we didn't want user output for those types of failures

would need to look into the PRs that added them

[vale ibex]

There's always a debug line

[static canyon]

Yeah, true

[static canyon]

Hmm yes, true. I guess I'm happy then

[outer oasis]

I do not
I was just wondering if you could leave the merge without fixing all the conflicts
I guess you have to fix them all before you can do anything with the branches anyways

[static canyon]

Also, what's the correct place to implement bot#2486. The error technically doesn't have to come from a command, so should I be putting in on_error or something? @vale ibex

[dusky shore]

GitHub

[bot] #2486 Forbidden: 403 Forbidden (error code: 90001): Reaction blocked

[static canyon]

Does on_error even run if it's from a command? If not I guess it'd have to go in both

[vale ibex]

seems fine to me

[vale ibex]

it should do

[atomic ivy]

the only way I know is you could just restore/checkout/reset the rest of the files (which would actually "resolving" the conflicts in those)
then carry on with the commit as usual

[static canyon]

Cool, will implement in on_error and do some testing 👍

on_error doesn't currently exist, so what do we want to do when it's unhandled? Same as in on_command_error (i.e. send the "an unexpected error happened, please let us know" message)?

That probably makes sense

[vale ibex]

it should exist in bot.py

it's what pushes stuff to sentry

Yea, that error seems fine. You'd want to check that it isn't handled elsewhere first though

[static canyon]

👍

It looks like on_error is only called for errors in event listeners btw, not commands

    async def on_error(self, event: str, *args, **kwargs) -> None:
        """Log errors raised in event listeners rather than printing them to stderr."""
        self.stats.incr(f"errors.event.{event}")

        with push_scope() as scope:
            scope.set_tag("event", event)
            scope.set_extra("args", args)
            scope.set_extra("kwargs", kwargs)

            log.exception(f"Unhandled exception in {event}.")```I assume we only want the pre-existing code (including increase the stat) to run when the error is unhandled (i.e. not this error)?

@vale ibex ^

It's also rather hard to give an feedback to Discord, since there's definitive channel or member or anything to send a message to. So I guess we just log

[static canyon]

bot-bot-1  | 2023-05-04 21:19:11 | pydis_core.utils.scheduling | ERROR | Error in task Task-75 275279291648!
bot-bot-1  | Traceback (most recent call last):
bot-bot-1  |   File "/bot/bot/exts/moderation/incidents.py", line 361, in crawl_incidents
bot-bot-1  |     await add_signals(message)
bot-bot-1  |   File "/bot/bot/exts/moderation/incidents.py", line 269, in add_signals
bot-bot-1  |     await incident.add_reaction(signal_emoji.value)
bot-bot-1  |   File "/opt/poetry/cache/virtualenvs/bot-TFcQMFAJ-py3.11/lib/python3.11/site-packages/discord/message.py", line 1098, in add_reaction
bot-bot-1  |     await self._state.http.add_reaction(self.channel.id, self.id, emoji)
bot-bot-1  |   File "/opt/poetry/cache/virtualenvs/bot-TFcQMFAJ-py3.11/lib/python3.11/site-packages/discord/http.py", line 738, in request
bot-bot-1  |     raise Forbidden(response, data)
bot-bot-1  | discord.errors.Forbidden: 403 Forbidden (error code: 90001): Reaction blocked```What error handler would I use for schedulers? It doesn't call `discord.on_error` (presumably because it's not a discord event directly triggering it)

[static canyon]

Sadly not

[static canyon]

Have PRd a draft under bot#2580, but still not sure how to handle this correctly

[dusky shore]

GitHub

[bot] #2580 Handle adding reactions to messages from users who have blocked the bot

[clever wraith]

hi so this my first time creating an issue there did i do everything correctly?

https://github.com/python-discord/sir-lancebot/issues/1266

also when can i work on the bug as it looks simple to solve do i need to get assigned or something?

[static canyon]

The issue looks good, yeah. As for when you can implement, we generally recommend not starting it until the issue has been approved

Otherwise you might put effort into something we don't want / want implemented differently / etc.

In this case, the help command logic is going to be moved to bot-core anyway, so it's somewhat defunct editing here in lancebot

[static canyon]

But give it some time and see what the core devs think. They may still approve it

[clever wraith]

👍

[static canyon]

Can I get some help with handling errors raised in schedulers? Not sure where the handling code would go (ideally it should be global to all schedules)

[clever wraith]

The handling should be done in each relevant async function if you know where it's raised from.

We're only wrapping the main event loops scheduling system

[static canyon]

But if I do it in the individual thing then it'll need to be added to all the others manually

I'm wanting a global thing, like on_command_error

So that it just works

[clever wraith]

What's all the others ?

What's the issue you're having exactly?

[static canyon]

I'm wanting to have to handle this in as few places as possible

[static canyon]

I've got it in on_error, which covers all events, and on_command_error which covers all commands.

This means we don't have to worry about finding all the pre-existing cases in events/commands, and don't have to worry about handling in any future events/commands.

However, we also need to handle the error inside schedules (at a minimum, the incident crawler schedule). If I put the handling in the incident crawler, as you suggest, then that means any other pre-existing/future schedules need to manually add the handling too. Ideally I'd have a global schedule-handler, like with events/commands

[gritty wind]

The error handling for a large portion of global errors is unique to the bot it’s for (bot has custom handlers which fork off, tags, silence, eval, lance has cooldown stuff, game-related stuff, redirecting to certain channels)

This is not something that can be merged except for the most simple of cases (which are already a line or two)

I don’t think bot-core needs to be a magic all-encompassing seamless tool which works for anything

So we don’t need to shove error handlers into it just because

[static canyon]

I'm not referring to bot-core itself, sorry, should've been clearer

I'm referring to handling bot#2486

[dusky shore]

GitHub

[bot] #2486 Forbidden: 403 Forbidden (error code: 90001): Reaction blocked

[static canyon]

I just want a global handler, within bot. If it can also go in bot-core to be applied to lancebot, then great, but that's not strictly what I'm after

I'm only wanting to handle this one error here (not even all discord.Forbidden)

My current implementation (bot##2580) handles all commands and all events, but not the schedules. I'm asking how to handle errors raised in schedules.

@gritty wind ^

[vale ibex]

Do they all use the bot core scheduler?

[vale ibex]

if so, you can use suppressed_exceptions in create_task is supress it

[static canyon]

Hmm, it's not just the exception class though

I.e. not all discord.Forbidden errors, but where the .code == 9001 or w/e it is

So I'm not sure if that'll work

Will take a look though

[vale ibex]

ah right, you could add support for custom callbacks in the botcore scheduler then

[static canyon]

Yeah

I'll look into it, thanks

[vale ibex]

np, feel free to split that out from your current pr though

[static canyon]

!remind 3h ^

[stable mountain]

Yeah okay.

Your reminder will arrive on <t:1683414645:F>!

ID: 5878

[static canyon]

Yeah, I probably will do

[static canyon]

Should I create an issue for it, or just go and PR?

Implementation seems really easy looking at the code

tuple[type[Exception] | Callable[[type[Exception]], bool], ...]```should be the correct typehint for "a tuple of exceptions, or callbacks that take an en exception and return a bool", right? And it can allow both in the same tuple?

[vale ibex]

sure you can PR

[vale ibex]

that doesn't seem right, better to make a new kwarg for a list of callbacks

[static canyon]

Yeah, just realised that when going to implement it

[static canyon]

def _log_task_exception(
    task: asyncio.Task,
    *,
    suppressed_exceptions: tuple[type[Exception], ...],
    suppressed_exception_callbacks: tuple[typing.Callable[[Exception], bool], ...]
) -> None:
    """Retrieve and log the exception raised in ``task`` if one exists and it's not suppressed."""
    with contextlib.suppress(asyncio.CancelledError):
        exception = task.exception()
        # Log the exception if one exists and it's not suppressed.
        if all((
            exception,
            not isinstance(exception, suppressed_exceptions),
            not any(callback(exception) for callback in suppressed_exception_callbacks)
        )):
            log = logging.get_logger(__name__)
            log.error(f"Error in task {task.get_name()} {id(task)}!", exc_info=exception)
``` @vale ibex do we want to add a `log.debug(f"Error in task {task.get_Name()} {id(task)} was suppressed")` or something like that, so we still get logging for tasks that fail but then get suppressed? Could help with debugging

[vale ibex]

I was expecting you to add the calbacks kwarg directly to create_task and then just add those as callbacks directly onto the task

minimal change to bot core and a lot more flexibility in use cases

[static canyon]

Yeah, I am

But it still needs to be in _log_task_exception since that's where those kwargs get handled

Well

def create_task(
    coro: abc.Coroutine[typing.Any, typing.Any, TASK_RETURN],
    *,
    suppressed_exceptions: tuple[type[Exception], ...] = (),
    suppressed_exception_callbacks: tuple[typing.Callable[[Exception], bool], ...] = (),
    event_loop: typing.Optional[asyncio.AbstractEventLoop] = None,
    **kwargs,
) -> asyncio.Task[TASK_RETURN]:
    """
    Wrapper for creating an :obj:`asyncio.Task` which logs exceptions raised in the task.

    If the ``event_loop`` kwarg is provided, the task is created from that event loop,
    otherwise the running loop is used.

    Args:
        coro: The function to call.
        suppressed_exceptions: Exceptions to be handled by the task.
        suppressed_exception_callbacks: Callbacks to be handled by the task.
        event_loop (:obj:`asyncio.AbstractEventLoop`): The loop to create the task from.
        kwargs: Passed to :py:func:`asyncio.create_task`.

    Returns:
        asyncio.Task: The wrapped task.
    """
    if event_loop is not None:
        task = event_loop.create_task(coro, **kwargs)
    else:
        task = asyncio.create_task(coro, **kwargs)

    _background_tasks.add(task)
    task.add_done_callback(_background_tasks.discard)
    task.add_done_callback(
        partial(
            _log_task_exception,
            suppressed_exceptions=suppressed_exceptions,
            suppressed_exception_callbacks=suppressed_exception_callbacks
        )
    )
    return task


def _log_task_exception(
    task: asyncio.Task,
    *,
    suppressed_exceptions: tuple[type[Exception], ...],
    suppressed_exception_callbacks: tuple[typing.Callable[[Exception], bool], ...]
) -> None:
    """Retrieve and log the exception raised in ``task`` if one exists and it's not suppressed."""
    with contextlib.suppress(asyncio.CancelledError):
        exception = task.exception()
        # Log the exception if one exists and it's not suppressed.
        if all((
            exception,
            not isinstance(exception, suppressed_exceptions),
            not any(callback(exception) for callback in suppressed_exception_callbacks)
        )):
            log = logging.get_logger(__name__)
            log.error(f"Error in task {task.get_name()} {id(task)}!", exc_info=exception)

[vale ibex]

the callbacks don't need to be just for suppressed exceptions

for callback in callbacks:
   task.add_done_callback(callback)

[static canyon]

Right, but then it'll still call the _log_task_exception and error

[vale ibex]

in this specific case for ignoring this error, you'd add the error to suppressed_exceptions so that _log_task_exception doesn't log it, and then handle what you want to happen in the custom callback

doing it this way means that the callback can be whatever the bots want

[static canyon]

you'd add the error to suppressed_exceptions so that _log_task_exception doesn't log it
But as I mentioned above, that's not possbile

Because I don't want to ignore all of a given exception

Only a subset of them

[vale ibex]

Yea, so you tell _log_task_exception to ignore all of them, and then do the error handling in the custom callback you will write in the bot

[static canyon]

Eh, I don't like that

I mean 🤔

[vale ibex]

supporting custom callbacks makes it far more flexible in the future

[static canyon]

In bot I already have```py
async def try_handle_forbidden(error: Forbidden, message: Message | None = None) -> bool:
"""Attempts to handle discord.Forbidden errors, returning True if successful else False."""
if error.code == 90001:
# Occurs when the bot attempted to add a reaction
# to a message from a user that has blocked the bot.
log.debug("Failed to add reaction to a message since the message author has blocked the bot")
if message:
await message.channel.send(
f":x: {message.author.mention} failed to add reactions to your message as you've blocked me.",
delete_after=30
)

    return True
return False```so I was going to just pass that in to the bot-core thing that I just wrote

[vale ibex]

so you'd just add raise error instead of returning false

[static canyon]

But then I need to edit the bot code that uses this function

bot#2580

[dusky shore]

GitHub

[bot] #2580 Handle adding reactions to messages from users who have blocked the bot

[static canyon]

            elif isinstance(e.original, Forbidden):
                if await try_handle_forbidden(e.original, ctx.message):
                    # Forbidden error was handled so return
                    return
                await self.handle_unexpected_error(ctx, e.original)```e.g. this in the `on_command_error`

If I re-raise then that won't work

[vale ibex]

elif isinstance(e.original, Forbidden):
    try:
        await try_handle_forbidden(e.original, ctx.message):
    except Forbidden:
        await self.handle_unexpected_error(ctx, e.original)

[static canyon]

Hmm

That's not too bad tbf

        if isinstance(e_val, Forbidden):
            event_to_message_indx = {
                "on_message": 0,
                "on_message_edit": 1
            }
            message = None
            if (message_indx_in_args := event_to_message_indx.get(event)) is not None:
                message = args[message_indx_in_args]

            if await try_handle_forbidden(e_val, message):
                # Error was handled so return
                return

        self.stats.incr(f"errors.event.{event}")

        with push_scope() as scope:
            scope.set_tag("event", event)
            scope.set_extra("args", args)
            scope.set_extra("kwargs", kwargs)
            log.exception(f"Unhandled exception in {event}.")```I guess it'd work here too

[vale ibex]

I'm also not convinced that doing this is worth the maintenance cost to just give an error message to the user

I'd much rather just log it and not send anything to Discord

[static canyon]

I was debating that, but it seems bad UX to not notify the user. Like posting a #incidents and then just getting no response what so ever

[vale ibex]

The user has already blocked the bot, so they're not going to see the message anyway

[static canyon]

It still shows that the bot sent a message though

You get the "1 blocked message" or w/e, which you can click on to view the content

[vale ibex]

I think it was just the code being harder to read that was giving me doubts, I've left a review

[static canyon]

Will look in a sec

!remind 10M ^

[stable mountain]

Affirmative!

Your reminder will arrive on <t:1683408784:F>!

ID: 5879

[static canyon]

bot-core#177

[dusky shore]

GitHub

[bot-core] #177 Add done_callbacks param to create_task

[stable mountain]

It has arrived!

Here's your reminder: ^
[Jump back to when you created the reminder](#dev-contrib message)

[stable mountain]

It has arrived!

Here's your reminder: ^
[Jump back to when you created the reminder](#dev-contrib message)

[static canyon]

Fyi realised I'd implemented bot-core#177 incorrectly, so have now fixed that @vale ibex @timid sentinel.

Tl;Dr; instead of done_callbacks argument, I moved handle_forbidden_from_block into bot-core and edited _log_task_exception to call that for Forbidden errors (which means that all schedules will have 90001 Forbidden errors handled by default -- no need to do anything from bot)

[dusky shore]

GitHub

[bot-core] #177 Handle discord.Forbidden 90001 errors by default in create_task()

[vale ibex]

alright cool, let me kow when it's ready to review

[static canyon]

It should be ready for review now

The bot one is waiting for bot-core, and bot-core is ready

Well, I've not updated the PR description but the actual code is good to be reviewed

[vale ibex]

it's failing lint and build

will ned a bump to version and a changelog too

[static canyon]

Ah, okay.

[static canyon]

Yeah, forgot about that

I'll work on that soon™️ and ping you after

[vale ibex]

Yea no rush

[static canyon]

Would it be a patch or minor bump? I can never remember the difference 😅

[vale ibex]

major = breaking, minor = feature, patch = bug fix

[static canyon]

Right, so minor then

Does botcore not have precommit hooks?

[clever wraith]

It does

You need to install it first

[static canyon]

How?

[clever wraith]

Poetry run task precommit

[static canyon]

That gives an error saying task isn't found

[clever wraith]

Because taskipy isn't installed, i presume

Can you list your dependencies?

[static canyon]

I probably need to poetry install first then?

[clever wraith]

Didn't you do that?

[static canyon]

Don't think so lol

Will try in a few mins

[clever wraith]

I'm confused, weren't you working on a bot core feature?

[static canyon]

Yeah

Doing poetry install fixed it, so it's working now

I did actually get an error: • Installing furo (2022.12.7): Failed at /opt/homebrew/Cellar/poetry/1.4.1/libexec/lib/python3.11/site-packages/installer/sources.py:289 in validate_record 285│ f"In {self._zipfile.filename}, hash / size of {item.filename} didn't match RECORD" 286│ ) 287│ 288│ if issues: → 289│ raise _WheelFileValidationError(issues) 290│ 291│ def get_contents(self) -> Iterator[WheelContentElement]: 292│ """Sequential access to all contents of the wheel (including dist-info files).but assume I can just ignore that

Erm 🤔py ______________________________________________________ ERROR collecting tests/pydis_core/utils/test_regex.py ______________________________________________________ ImportError while importing test module '/Users/srn42/Documents/personal/bot-core/tests/pydis_core/utils/test_regex.py'. Hint: make sure your test modules/packages have valid Python names. Traceback: /opt/homebrew/Cellar/python@3.11/3.11.2_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/importlib/__init__.py:126: in import_module return _bootstrap._gcd_import(name[level:], package, level) tests/pydis_core/utils/test_regex.py:4: in <module> from pydis_core.utils.regex import DISCORD_INVITE E ModuleNotFoundError: No module named 'pydis_core'

[clever wraith]

How're you executing this ?

[clever wraith]

I meant this*

[static canyon]

poetry run task test

[timid sentinel]

This is probably the issue, try running poetry install again, does it continue to happen?

[static canyon]

Yeah

furo fails every time

[clever wraith]

Is there more to the exception when installing furo ?

[static canyon]

https://paste.pythondiscord.com/lofupuvuro.py

That's the full output

[timid sentinel]

hm, try poetry run pip install furo==2022.12.7 and then poetry install again maybe?

not sure what the actual issue is, but that might avoid it

[static canyon]

That... seems to work

And the testing issue is fixed too :)

Thanks 😄

Also

If I made a non-async function async, does that count as a breaking change?

I guess so, since calls to the function have to be changed

Warning, treated as error:
/home/runner/work/bot-core/bot-core/docs/output/pydis_core.utils.rst:12:toctree contains reference to nonexisting document 'output/pydis_core.utils.errors'
https://github.com/python-discord/bot-core/actions/runs/4908330910/jobs/8764035477

pydis_core.utils.errors is a file that I created as part of the PR. Do I have to do something to make the build docs step recognise it?

Looks like I have to add it to the __all__ of pydis_core.utils.__init__

Yeah, that seems to have fixed it

[clever wraith]

Yes, which function is that ?

Since calling those now in all the other code will just making a coroutine

And it won't have any effect unless it's awaited

Is the change in your PR ?

[static canyon]

Yeah, bot-core#177

[dusky shore]

GitHub

[bot-core] #177 Handle discord.Forbidden 90001 errors by default in create_task()

[static canyon]

It only had one usage, which was in the same file

[clever wraith]

Yeah, the function is private, so that should only be referenced in bot-core

However, these conventions aren't always 'respected', so might as well verify

Is it in the module where we have the __all__ defined ?

[static canyon]

Err

It's in /utils/scheduling.py, which doesn't have an __all__, but /utils.__init__.py does (and includes scheduling)

It is technically possible to import the private function if that's what you're wondering

[clever wraith]

Yes yes I know, python doesn't enforce these things

Which is why I said they're not always respected

And it's better to verify

[static canyon]

@vale ibex bot-core#177 is now passing lint & build and is ready for review :)

[dusky shore]

GitHub

[bot-core] #177 Handle discord.Forbidden 90001 errors by default in create_task()

[static canyon]

Have also updated the PR description

[clever wraith]

how long does it usually take to merge PR?

[clever wraith]

There's no defined duration.
It needs to be approved by staff and/or core devs, and once approved, it'll get Merged

[clever wraith]

okay thanks

[clever wraith]

If you have a PR and it goes stale, you can ping us here to review it

[static canyon]

Fyi @eternal owl have addressed your defaultdict comment on bot#2520, thanks :)

[dusky shore]

GitHub

[bot] #2520 Handle off-topic channel names that are invalid for servers in Server Discovery

[midnight jasper]

From experience a couple days to a couple weeks.

[midnight jasper]

How do you get started again for contributing to python bot?

[royal prawn]

!contrib

[stable mountain]

Contribute to Python Discord's open source projects

Looking to contribute to Open Source Projects for the first time? Want to add a feature or fix a bug on the bots on this server? We have on-going projects that people can contribute to, even if you've never contributed to open source before!

Projects to Contribute to
• Sir Lancebot - our fun, beginner-friendly bot
• Python - our utility & moderation bot
• Site - resources, guides, and more

Where to start

1. Read our contribution guide
2. Chat with us in #dev-contrib if you're ready to jump in or have any questions
3. Open an issue or ask to be assigned to an issue to work on

[fiery bay]

Hi

https://github.com/python-discord/sir-lancebot/issues/143

can i try to work on this issue?

[clever wraith]

Sure thing, can you comment on that PR that you want to take it on ? And I'll assign you

[atomic ivy]

!rule 7 9

[stable mountain]

Rules

7. Keep discussions relevant to the channel topic. Each channel's description tells you the topic.

9. Do not offer or ask for paid work of any kind.

[fiery bay]

okay, i will comment in 1-2 days

[rapid igloo]

hi, is it possible to get the fontawesome icons for local development?

I've looked at the networks tab and it seems to not allow requests to https://kit.fontawesome.com/ff22cb6f41.js since the origin is not the public www.pythondiscord.com

does this have to do with staticfiles? If so I've also tried a bunch of combinations of settings including actually using the default /app/staticfiles on a gitpod environment with no luck

(probably doesnt help) I think it's even worse if I try static builds - no favicon - although that is probably a separate issue that I could fix

[gritty wind]

The site uses premium icons, so we can’t do much there

[tawdry vapor]

Can we conditionally swap out premium icons for free versions if we detect premium is not available?

Surely we can find free icons that look close enough for the purposes of local dev

[gritty wind]

Possibly, but it wouldn’t quite be worth the effort imo considering our setup

One page I know is particularly egregious is contributing, which is a markdown static file

Getting a switch into that doesn’t sound fun

In b4 we use regex to parse and update the html with the correct url

[tawdry vapor]

Oh, we don't use HTML templates anymore?

Cause I was thinking something like <img src={getIcon("premium-icon-id")}/> and the function just falls back to a non-premium one.

[rapid igloo]

Thanks! I see something relate here https://github.com/python-discord/site/blob/main/static-builds/README.md?plain=1#L55-L57

Just curious, how do you guys configure netlify for the github actions preview to use the premium icons, since the build-previews arent on www.pythondiscord.com either?

[stable mountain]

static-builds/README.md?plain=1 lines 55 to 57

Note that at this time, if you are deploying to netlify yourself, you won't have access to the
fa-icons pack we are using, which will lead to many missing icons on your preview.
You can either update the pack to one which will work on your domain, or you'll have to live with the missing icons.```

[gritty wind]

Fa allows you to specify multiple domains under one subscription

So we have our main site and netlify here

[gritty wind]

We do in some places, but a lot of content is just static HTML

[clever wraith]

looks like some of the redirect urls have not been updated and it caused 404 NotFound https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/contributing-guidelines/commit-messages/

looks like its caused by the rename of the contributing folder

i see there was already issue about it (site#809) but im still getting 404 should i make new issue ?

[dusky shore]

GitHub

[site] #809 Contributing Guidelines-Writing Good Commit Messages Page Not Found

[cold island]

Where are you accessing this link from?

Because the link in the contrib guidelines works

[clever wraith]

https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/#5-make-changes

here

[static canyon]

The "how to write good commit messages" link 404s for me too

This is a different hyperlink that was missed in site#810

[dusky shore]

GitHub

[site] #810 Fix 404 for commit message document

[cold island]

Yeah it should be updated to the one that works

[eternal owl]

I dont think I can commit directly to this branch right ? I should make a new PR ig, or is there a workaround

https://github.com/python-discord/site/pull/702

[outer oasis]

You should be able to

owow

Year old PR

[sharp crag]

Yeah it's kinda stale - picking it back up now

[mellow hare]

Is that normal?

Other list methods show up fine

[fierce jewel]

!faq

[stable mountain]

Frequently asked questions

As the largest Python community on Discord, we get hundreds of questions every day. Many of these questions have been asked before. We've compiled a list of the most frequently asked questions along with their answers, which can be found on our FAQ page.

[fierce jewel]

Hey folks, is there a faq for the contribution ? basically i want to know if i can start working on a github issue without being assigned to it ?

[static canyon]

!contributing is the closest we have

[stable mountain]

Contribute to Python Discord's open source projects

Looking to contribute to Open Source Projects for the first time? Want to add a feature or fix a bug on the bots on this server? We have on-going projects that people can contribute to, even if you've never contributed to open source before!

Projects to Contribute to
• Sir Lancebot - our fun, beginner-friendly bot
• Python - our utility & moderation bot
• Site - resources, guides, and more

Where to start

1. Read our contribution guide
2. Chat with us in #dev-contrib if you're ready to jump in or have any questions
3. Open an issue or ask to be assigned to an issue to work on

[static canyon]

But tl;dr; no -- you have to be assigned first

The basically flow would be something like:

Issue created --> issue approved --> user requests to work on it --> user gets assigned --> user creates PR --> PR gets reviewed --> PR gets merged and issue is closed

[fierce jewel]

Got it thanks @static canyon , yet not sure if i will be capable of finishing the issue in a timely manner, will it hurt if i abandon it ?

[static canyon]

There's no real time limit on PRs, so just work on it when you can

If you do abandon it then it causes issues because people have to start over usually (since they're unable to commit to your forked repo branch)

[fierce jewel]

aha, okay, thanks for the clarification, will tackle a smaller issue to avoid the latter happening 🙂

[eternal owl]

I pushed changes rn @sharp crag , hopefully we can get it merged in after yours and @brisk brook review
I did make quite a few changes

site#702

[dusky shore]

GitHub

[site] #702 Subclassing bot

[sharp crag]

Thanks! I'll review when I'm home

!remindme 3h this^

[stable mountain]

You got it!

Your reminder will arrive on <t:1684270857:F>!

ID: 5890

[clever wraith]

(It's a faster feedback loop here :P)

def get_launch_time_str(self) -> datetime.datetime:
   """Get bot launch datetime without milliseconds in UTC and status."""
   return f"Bot started at: {self.launch_time.strftime('%F %T')} UTC."

The return types & annotations don't match, can you fix that please ?

[eternal owl]

ah fak

[clever wraith]

No worries

[eternal owl]

ty for pointing it out btw o/

[clever wraith]

Sure thing

Also, the Bot highlighting in the title didn't go through 😛

[eternal owl]

## The Benefits of Subclassing Bot this line ?

cant find it, could you give me the line number

[clever wraith]

line 3 😄

[eternal owl]

got it chief

[clever wraith]

Thaaank you !

The only thing left is to update this part

With the above example, you are not required to change any of the existing or future code, it is identical to code done without subclassing bot.

and we should be ready to

[eternal owl]

I feel that line is irrelevant tho, as you can do the same stuff even after subclassing, the only difference is the additional functionality that was added with subclassing
I could just paraphrase that sentence

[clever wraith]

I'm just a bit skeptical about the '' you don't have to change anything '' part

The only thing that needs changing is the instantiation.

Also, going from regular Bot class to custom requires only that change.

But moving backwards becomes problematic when you couple your app/cogs to the custom methods

[eternal owl]

how about

Following the above example, your existing code should still function unless you have overridden any of the discord.py's method without retaining its functionality (its good to call super().method(*args, **kwargs) when overriding something unless you know what your doing). The new features added via subclasses can now be leverage in your existing commands/features.```

rough outline @clever wraith

maybe you can paraphrase it better than me, lol

[clever wraith]

I'm actually now thinking whether we should mention this at all

Ahaha

[eternal owl]

I think its useful to mention to always call super() when overriding methods, just in case

[clever wraith]

Yes but that's basic OOP and not specific to subclassing bot

[eternal owl]

right

[eternal owl]

if we are considering this, then we can just remove that line

[clever wraith]

I think so.

We can see what others think too

[eternal owl]

alr

[clever wraith]

Thanks for picking this up btw, it has been sleeping for a while

[eternal owl]

no worries o/

[stable mountain]

It has arrived!

Here's your reminder: this^
[Jump back to when you created the reminder](#dev-contrib message)

[sharp crag]

Done ^.^
Bella covered most of it :3

[clever wraith]

Shall we remove that part then? ( if you agree ofc)
And we can then merge it

[eternal owl]

alr, I will mention about cogs in about a line tho

[clever wraith]

Go ahead

[eternal owl]

eventually we could release a guide on cogs, but for now I will link to external resources

[clever wraith]

Don't we have that?

Or at least an issue for it

[eternal owl]

right, there is an issue open for it being a tag

site#901

[dusky shore]

GitHub

[site] #901 New tag on using cogs/extensions in discord.py

[eternal owl]

its kinda stalled atm

[still comet]

my bad, it should have been a page and not a tag, I'll go change that now

cogs/extensions would be way too tight to fit in a tag

[eternal owl]

ah okay

[clever wraith]

@eternal owl You've requested my review but it seems you've missed my last comment

[eternal owl]

yikes did not push

done

[clever wraith]

@eternal owl The new link is broken.
Weird, I tested it like that locally & it worked.
I remember @vale ibex and I having a talk about how site is rendering paths to find pages

[eternal owl]

I tested it

it worked tho

[clever wraith]

I tested it too

But it doesn't work on the preview

That looks like the right path tho

I'm not next to my pc now, so can't verify that

[vale ibex]

there shouldn't be an .md

[vale ibex]

how did you test it?

running the site locally and clicking the link?

[eternal owl]

runnning the server ?