Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(guild): add bulk_ban function #1172

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

feat(guild): add bulk_ban function #1172

wants to merge 16 commits into from

Conversation

spifory
Copy link
Collaborator

@spifory spifory commented Mar 22, 2024 •
edited by EmreTech

Summary

Implements discord/discord-api-docs#6720 by adding a bulk_ban function to Guild

This is a Code Change

  • I have tested my changes.
  • I have updated the documentation to reflect the changes.
  • I have run task pyright and fixed the relevant issues.

@spifory spifory added s: in progress Status: the issue or PR is in development/progress p: medium Priority: medium - should be worked on in the near future t: api coverage Type: api coverage - this adds code to cover the discord API s: waiting for discord status: the issue requires changes to the documentation or behaviour before it can be completed. labels Mar 22, 2024
@spifory spifory marked this pull request as draft March 22, 2024 11:18
@spifory
style: un-format Guild.afk_channel
baaaa6f 
Ctrl + S must've formatted the whole file, this is irrelevant to what I'm adding
teaishealthy
teaishealthy requested changes Mar 22, 2024
View reviewed changes
Copy link
Collaborator

@teaishealthy teaishealthy left a comment

Choose a reason for hiding this comment

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

code looks good

If all users "failed" you get an error response instead

but i feel like this is a bit too unexpected, should go under a Raises: HTTPException: ... (?)

EmreTech
EmreTech requested changes Mar 28, 2024
View reviewed changes
Copy link
Collaborator

@EmreTech EmreTech left a comment

Choose a reason for hiding this comment

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

Code looks good otherwise!

nextcord/guild.py Outdated Show resolved Hide resolved
nextcord/http.py Show resolved Hide resolved
nextcord/guild.py Outdated Show resolved Hide resolved
nextcord/guild.py Outdated Show resolved Hide resolved
nextcord/guild.py Outdated Show resolved Hide resolved
nextcord/guild.py Show resolved Hide resolved
nextcord/bans.py Show resolved Hide resolved
@EmreTech EmreTech removed the s: waiting for discord status: the issue requires changes to the documentation or behaviour before it can be completed. label May 18, 2024
@EmreTech
Copy link
Collaborator

Related Discord Docs PR was merged.

@EmreTech EmreTech marked this pull request as ready for review May 18, 2024 01:24
@EmreTech EmreTech added s: awaiting review Status: the issue or PR is awaiting reviews and removed s: in progress Status: the issue or PR is in development/progress labels May 18, 2024
EmreTech
EmreTech reviewed May 22, 2024
View reviewed changes
Copy link
Collaborator

@EmreTech EmreTech left a comment

Choose a reason for hiding this comment

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

Looks good otherwise!

nextcord/guild.py Show resolved Hide resolved
@spifory spifory requested a review from EmreTech May 23, 2024 05:15
teaishealthy
teaishealthy reviewed May 23, 2024
View reviewed changes
nextcord/guild.py
The number of seconds worth of messages to delete from these users.
Can range from 0 to 604800 seconds (7 days).

Defaults to 0.
Copy link
Collaborator

Choose a reason for hiding this comment

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

why does the default differ from the the Guild.ban default

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had seen For backwards compatibility reasons, by default one day worth of messages will be deleted. in ban's documentation so I assumed if not for backwards compatibility it would've been 0 anyways

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teaishealthy teaishealthy teaishealthy requested changes

@EmreTech EmreTech Awaiting requested review from EmreTech

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
p: medium Priority: medium - should be worked on in the near future s: awaiting review Status: the issue or PR is awaiting reviews t: api coverage Type: api coverage - this adds code to cover the discord API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@spifory @EmreTech @teaishealthy