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(iterators): add GuildIterator.with_counts #1093

Merged
merged 6 commits into from Aug 25, 2023
Merged

feat(iterators): add GuildIterator.with_counts #1093

merged 6 commits into from Aug 25, 2023

Conversation

spifory
Copy link
Collaborator

@spifory spifory commented Jul 18, 2023

Summary

Implements discord/discord-api-docs#5628

In case you're wondering, "why is the default for with_counts False?"
Well that is because the default on the documentation false, so just for the sake of consistency, that is why the default is False.

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 force-pushed the spifory/fix/add-with-counts-param branch 2 times, most recently from 855311e to 405e5f6 Compare July 18, 2023 07:35
@spifory spifory force-pushed the spifory/fix/add-with-counts-param branch from 08b270c to 984b1c5 Compare July 18, 2023 07:37
@spifory
Copy link
Collaborator Author

spifory commented Jul 18, 2023

I accidentally made this branch off of another branch, and not master, so that is the reason for the force-pushes

@spifory spifory added this to the 2.6 milestone Jul 18, 2023
@spifory spifory added t: enhancement Type: enhancement - new feature or request 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 2.6 The issue/PR should go for the 2.6 release labels Jul 18, 2023
@spifory
fix(iterators): change with_guilds to with_counts
c88bdfe 
I'm not even sure what happened here
teaishealthy
teaishealthy requested changes Jul 26, 2023
View reviewed changes
nextcord/iterators.py Outdated Show resolved Hide resolved
@spifory @teaishealthy
docs(iterators): change default to False
f43712d 
Co-authored-by: teaishealthy <76410798+teaishealthy@users.noreply.github.com>
teaishealthy
teaishealthy approved these changes Jul 26, 2023
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.

is probably ok 🚀

@ooliver1 ooliver1 removed the t: enhancement Type: enhancement - new feature or request label Aug 10, 2023
EmreTech
EmreTech requested changes Aug 12, 2023
View reviewed changes
nextcord/iterators.py Outdated Show resolved Hide resolved
nextcord/iterators.py Outdated Show resolved Hide resolved
nextcord/iterators.py Outdated Show resolved Hide resolved
nextcord/iterators.py Outdated Show resolved Hide resolved
@spifory @EmreTech
refactor!: use with_counts passed from ReactionIterator instead
220ea60 
Co-authored-by: Emre Terzioglu <50607143+EmreTech@users.noreply.github.com>
@spifory spifory requested a review from EmreTech August 13, 2023 09:05
EmreTech
EmreTech requested changes Aug 13, 2023
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.

This PR fails testing. I got an exception that yarl (which aiohttp uses for query parameters) doesn't support booleans.

You have to do something like HTTPClient.get_guild:

nextcord/nextcord/http.py

Line 1360 in a4bd5d2

params = {"with_counts": int(with_counts)}

@spifory
Copy link
Collaborator Author

spifory commented Aug 15, 2023

@EmreTech 244899b

@spifory spifory requested a review from EmreTech August 15, 2023 12:51
EmreTech
EmreTech approved these changes Aug 15, 2023
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.

Tested.

@ooliver1 ooliver1 merged commit a504662 into nextcord:master Aug 25, 2023
10 checks passed
@ooliver1 ooliver1 removed the s: awaiting review Status: the issue or PR is awaiting reviews label Aug 25, 2023
@spifory spifory deleted the spifory/fix/add-with-counts-param branch August 25, 2023 23:59
EmreTech added a commit to EmreTech/nextcord that referenced this pull request Sep 17, 2023
@EmreTech
feat: port nextcord#1093
679946d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ooliver1 ooliver1 ooliver1 approved these changes

@EmreTech EmreTech EmreTech approved these changes

@teaishealthy teaishealthy teaishealthy approved these changes

Assignees
No one assigned
Labels
2.6 The issue/PR should go for the 2.6 release 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
Projects
None yet
Milestone
2.6
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@spifory @ooliver1 @EmreTech @teaishealthy