I've also implemented some small caching in the botcore client to only make requests when necessary, and to only make one request to get all of the webhooks. While yes, webhooks are free, I left request logging enabled so the user could see what requests were made. I didn't see a good reason to silence those, both for transparency and because they already provide logging.

We also provide an invite link if the bot isn't in the guild already, but it is configured.

I've also added support which sets the app's flags to enable message content intents and member intent if they aren't already enabled.

closes#3414

onerandomusername mentioned this pull request

Oct 21, 2025

Support upgrading existing test guilds with botstrap#3418

Open

onerandomusername marked this pull request as ready for review

October 22, 2025 00:00

jb3 added the hacktoberfest-accepted label

Oct 22, 2025

onerandomusername force-pushed theqt/beginner-friendly-botstrap branch from496b553 toa0b56a6Compare

October 25, 2025 18:55

jb3 requested changes

Oct 25, 2025

View reviewed changes

Copy link

Member

jb3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Few review comments.

Can we also squash down the history a little bit? I don't think we need 21 commits for the changes here. Obviously keep the constants one separate but I think we can squash down a lot of the other ones.

botstrap.py OutdatedShow resolvedHide resolved

onerandomusername force-pushed theqt/beginner-friendly-botstrap branch 5 times, most recently fromf5f2c3c toa61eb26Compare

October 25, 2025 21:33

onerandomusername requested a review fromjb3

October 25, 2025 21:37

Copy link

ContributorAuthor

onerandomusername commentedOct 25, 2025

@jb3 I've compressed the history and addressed your comments in the updated history. Comments should now follow pep8, formatted with ruff, and silenced pyright.

jb3 requested changes

Oct 26, 2025

View reviewed changes

Copy link

Member

jb3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for addressing review feedback.

Only a couple of points left now, but also a couple of wider points:

I think thewith block we use now to run the script is too long and I think things like the webhook management & emoji management should be moved into their own methods that we then call (the with should be as small as possible really).
The existing webhook if statement is still a bit scary (I thought I'd left a review comment but can't see/find it now!), I'd prefer creating a new method along the lines offind_matching_webhook, something like:

deffind_matching_webhook(existing_webhooks,webhook_model,formatted_name,channel_id):forhookinexisting_webhooks:ifhook["id"]==str(webhook_model.id):returnhook["id"]ifhook["name"]==formatted_nameandhook["channel_id"]==str(channel_id):returnhook["id"]returnNone

It's all just become a little bit too complex to follow under that context manager and I think that some splitting up into smaller more digestible methods is a good idea here.

botstrap.py OutdatedShow resolvedHide resolved

jb3 requested changes

Oct 26, 2025

View reviewed changes

botstrap.py OutdatedShow resolvedHide resolved

jb3 requested changes

Oct 26, 2025

View reviewed changes

Copy link

Member

jb3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Sorry for these comments all being in parts, just keep doing scans and picking up new bits.

Can we also standardise on whether you're using f-strings in this file or using%s interpolation? There is a mix currently (I appreciate it was f-strings before which is not correct, but if you're adding some f-strings and some %s strings we may as well unify).

botstrap.py OutdatedShow resolvedHide resolved

onerandomusername added4 commits

October 27, 2025 10:33

refactor(logging): make botstrap logging useful

41d23e2

- provide completion message- silence httpcore logger- filter out botcore's warning for patching send_typing- use short url for contributing link- only use logging.warning and up for things that need user action- use logging.fatal when exiting due to error- use % for all logging statements

style: format with ruff, fix typehints, save magic values as constants

6b3b3ac

upgrade application flags if intents weren't enabled

b787bc1

provide an invite link if the bot isn't in the configured guild

b488508

onerandomusername force-pushed theqt/beginner-friendly-botstrap branch froma61eb26 to57c92b0Compare

October 27, 2025 14:38

onerandomusername added5 commits

October 27, 2025 10:43

refactor: cache guild info and guild channels

85665ee

this lowers the amount of requests we make and reduces the chance of ratelimiting

refactor: request all webhooks at once, match by name

f6b31cc

* request all webhooks in a single call rather than per configured webhook* Enable support for multiple identical test botssharing the same webhookThis should prevent the limit of 10 webhooks per channel from being reachedwithout needing to share a specific guild configuration for the PyDisstaff test guild

fix: only override the webhook ID for configuring webhooks

4e3fab0

this makes it easier to configure webhooks:we now only need to set their ID and not their channel.Setting the channel ID is only use for botstrap configuration, anyhow.

add audit log reason to requests that support it

3d44321

refactor: sync emojis to the test guild

82fa0b1

onerandomusername force-pushed theqt/beginner-friendly-botstrap branch from57c92b0 to82fa0b1Compare

October 27, 2025 19:37

refactor: rewrite Botstrap to be a class instead of one big-old if st…

084b45f

…atement

jb3 requested changes

Oct 27, 2025

View reviewed changes

botstrap.pyShow resolvedHide resolved

botstrap.py Outdated

Comment on lines 397 to 407

		if (
		# check the existing ID matches the configured one
		existing_hook["id"]==str(webhook_model.id)
		or (
		# check if the name and the channel ID match the configured ones
		existing_hook["name"]==formatted_webhook_name
		andexisting_hook["channel_id"]==str(all_channels[webhook_name])
		)
		):
		webhook_id=existing_hook["id"]
		break

Copy link

Member

jb3Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I have commented every review this needs changing 🥲.

Split out to a helper method without the complex conditions. Also, comments should always have a capital (and only be present if they actually add something, for simple conditionals they generally don't).

deffind_matching_webhook(existing_webhooks,webhook_model,formatted_name,channel_id):forhookinexisting_webhooks:ifhook["id"]==str(webhook_model.id):returnhook["id"]ifhook["name"]==formatted_nameandhook["channel_id"]==str(channel_id):returnhook["id"]returnNone

Copy link

ContributorAuthor

onerandomusernameOct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I sort of cleaned up this code but I'm not sure about the separate method with just how many specific inputs are required.

botstrap.py OutdatedShow resolvedHide resolved

botstrap.pyShow resolvedHide resolved

botstrap.py OutdatedShow resolvedHide resolved

botstrap.py Outdated

Comment on lines 467 to 468

		withbotstrap:
		botstrap.run()

Copy link

Member

jb3Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm not sure if this is the way this should be done or whether it'd be cleaner to create a DiscordClient here, pass it into the botstrap initialiser and then avoid having to define the__exit__ method for the BotStrapper class.

e.g.

withDiscordClient(guild_id=GUILD_ID)asdiscord_client:botstrap=BotStrapper(client=discord_client)

Copy link

ContributorAuthor

onerandomusernameOct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I would prefer to manage the DiscordClient internally on the botstrap class, even if we use contextlib to manage the context manager.

DiscordClient should not be used by anything other than the BotStrap class, and an instance of it outside of botstrap doesn't need to exist.

onerandomusername added2 commits

October 28, 2025 20:38

provide a helpful error message when BOT_TOKEN is not set, use pydant…

ea5c265

…ic for env management

apply suggestions from code review

f0cd9ce

jb3 requested changes

Oct 29, 2025

View reviewed changes

botstrap.py OutdatedShow resolvedHide resolved

botstrap.pyShow resolvedHide resolved

botstrap.py OutdatedShow resolvedHide resolved

jb3 requested changes

Oct 29, 2025

View reviewed changes

botstrap.py OutdatedShow resolvedHide resolved

decorator-factory reviewed

Oct 29, 2025

View reviewed changes

botstrap.py Outdated

		defcreate_forum_channel(self,channel_name_:str,category_id_:int\|str\|None=None)->str:
		"""Creates a new forum channel."""
		payload= {"name":channel_name_,"type":GUILD_FORUM_TYPE}
		payload:dict[str,Any]= {"name":channel_name_,"type":GUILD_FORUM_TYPE}

Copy link

Member

decorator-factoryOct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

nitpick:dict[str, object] (avoid Any unless really necessary)

Copy link

Member

jb3Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Any not necessary for these. +1

botstrap.py OutdatedShow resolvedHide resolved

onerandomusername added6 commits

October 28, 2025 22:42

remove special forum channel handling

a1facf2

interface updates, typehints, variable names

f83cd4d

fix: touch env_file if it doesn't exist

4236b13

don't use a property api request

438ca32

fix: add utf8 encoding for windows support

8eee160

Merge branch 'main' into qt/beginner-friendly-botstrap

075fc05

jb3 requested changes

Nov 2, 2025

View reviewed changes

botstrap.py

		log.info("Botstrap completed successfully. Updated configuration has been written to %s",ENV_FILE)
		else:
		log.info("Botstrap completed successfully. No changes were necessary.")
		sys.exit(changes_made)

Copy link

Member

jb3Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Why exit with an error if we make changes? Makes sense for a linter, not for this.

botstrap.py

Comment on lines +475 to +480

		withbotstrap:
		changes_made=botstrap.run()
		ifchanges_made:
		log.info("Botstrap completed successfully. Updated configuration has been written to %s",ENV_FILE)
		else:
		log.info("Botstrap completed successfully. No changes were necessary.")

Copy link

Member

jb3Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Space.

Suggested change

	withbotstrap:
	changes_made=botstrap.run()
	ifchanges_made:
	log.info("Botstrap completed successfully. Updated configuration has been written to %s",ENV_FILE)
	else:
	log.info("Botstrap completed successfully. No changes were necessary.")
	withbotstrap:
	changes_made=botstrap.run()

	ifchanges_made:
	log.info("Botstrap completed successfully. Updated configuration has been written to %s",ENV_FILE)
	else:
	log.info("Botstrap completed successfully. No changes were necessary.")

botstrap.py

		defget_emoji_contents(self,id_:str\|int)->bytes\|None:
		"""Fetches the image data for an emoji by ID."""
		# Emojis are located at https://cdn.discordapp.com/emojis/{emoji_id}.{ext}
		response=self.get(f"{self.CDN_BASE_URL}/emojis/{id_!s}.webp")

Copy link

Member

jb3Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Shouldn't fetch webp if you then submitimage/png, even if Discord is accepting it this doesn't seem right.

botstrap.py

		"""
		# Fetch first to modify, not overwrite
		current_flags=self.get_app_info().get("flags",0)
		new_flags=current_flags\|1<<15\|1<<19

Copy link

Member

jb3Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

No magic values here.

Labels

hacktoberfest-accepted

3 participants

Movatterモバイル変換

Uh oh!

refactor: update botstrap to be more beginner-friendly#3417

Are you sure you want to change the base?

refactor: update botstrap to be more beginner-friendly#3417

Uh oh!

Conversation

onerandomusername commentedOct 21, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

jb3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

onerandomusername commentedOct 25, 2025

Uh oh!

jb3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jb3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

onerandomusername commentedOct 21, 2025•
edited
Loading