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

Fix self._application_commands.pop in remove_application_command #1391

Merged
merged 5 commits into from
May 27, 2022
Merged

Fix self._application_commands.pop in remove_application_command #1391

merged 5 commits into from
May 27, 2022

Conversation

yoggys
Copy link
Contributor

@yoggys yoggys commented May 27, 2022

Summary

self._application_commands key types are strings, .pop(int(command.id), None) method doesn't affect any element of dict.
Change to .pop(command.id, None) will successfully remove the command from dict. This fixes issue #1390

image

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, typehinting, examples, ...)

Lulalaby
Lulalaby requested changes May 27, 2022
View reviewed changes
Copy link
Member

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

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

And please use a descriptive title instead of Fixes #xyz.

Instead write this into the description

discord/bot.py Outdated Show resolved Hide resolved
@yoggys yoggys changed the title Fix #1390 Fix self._application_commands.pop in remove_application_command May 27, 2022
@yoggys yoggys requested a review from Lulalaby May 27, 2022 10:00
Lulalaby
Lulalaby approved these changes May 27, 2022
View reviewed changes
Copy link
Member

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

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

Thank you! ❤️

@Lulalaby Lulalaby enabled auto-merge (squash) May 27, 2022 10:16
@Lulalaby Lulalaby linked an issue May 27, 2022 that may be closed by this pull request
bot.remove_application_command doesn't clear command remnants correctly #1390
Closed
3 tasks
@Lulalaby Lulalaby added Merge with squash bug Something isn't working priority: high High Priority status: awaiting review Awaiting review from a maintainer labels May 27, 2022
@Lulalaby Lulalaby requested review from krittick and Middledot May 27, 2022 10:17
@Lulalaby
Copy link
Member

@Pycord-Development/contributors
@Pycord-Development/maintainers

@Lulalaby Lulalaby merged commit 905b7d3 into Pycord-Development:master May 27, 2022
Lulalaby pushed a commit that referenced this pull request Jun 11, 2022
@yoggys @Lulalaby
Fix self._application_commands.pop in remove_application_command (#…
befaf62 
…1391)

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

@Dorukyum Dorukyum Dorukyum approved these changes

@Lulalaby Lulalaby Lulalaby approved these changes

@krittick krittick Awaiting requested review from krittick

@Middledot Middledot Awaiting requested review from Middledot

Assignees

@yoggys yoggys

Labels
bug Something isn't working priority: high High Priority status: awaiting review Awaiting review from a maintainer
Projects
Pycord Releases
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bot.remove_application_command doesn't clear command remnants correctly
3 participants
@yoggys @Lulalaby @Dorukyum