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.

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

feat: add MessageFlags.suppress_notifications and flags parameter #929

Merged
merged 11 commits into from Feb 21, 2023

Conversation

shiftinv
Copy link
Member

Summary

Adds MessageFlags.suppress_notifications (discord/discord-api-docs#5910), and implements a flags parameter on all send-like methods.

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running task lint
    • I have type-checked the code by running task pyright
  • 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, ...)

@shiftinv shiftinv added t: enhancement New feature t: api support Support of Discord API features s: needs review Issue/PR is awaiting reviews labels Feb 10, 2023
@shiftinv shiftinv added this to the disnake v2.9 milestone Feb 10, 2023
Copy link
Member

@EQUENOS EQUENOS left a comment

Choose a reason for hiding this comment

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

(Code review)
Lgtm! Should we deprecate suppress_embeds eventually?

@shiftinv
Copy link
Member Author

Should we deprecate suppress_embeds eventually?

Perhaps, yeah. suppress on Message.edit is already deprecated (was replaced by suppress_embeds), but there's no harm in keeping suppress_embeds for now, in my opinion.

Something I forgot to mention - this PR currently doesn't add a flags parameter to edit methods, just the send methods. Should we add that as well? It would make sense to add them all at once.

@onerandomusername
Copy link
Member

Something I forgot to mention - this PR currently doesn't add a flags parameter to edit methods, just the send methods. Should we add that as well? It would make sense to add them all at once.

Yeah, we should add them.

@EQUENOS
Copy link
Member

EQUENOS commented Feb 16, 2023

Yeah let's add this parameter to edit methods as well

@shiftinv
Copy link
Member Author

Yeah, we should add them.

Yeah let's add this parameter to edit methods as well

Alrighty, added in c83dfe9.

Copy link
Member

@EQUENOS EQUENOS left a comment

Choose a reason for hiding this comment

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

Lgtm, passed my tests as well.

disnake/abc.py Outdated Show resolved Hide resolved
@onerandomusername onerandomusername enabled auto-merge (squash) February 21, 2023 01:48
Copy link
Member

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

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

lgtm

@onerandomusername onerandomusername merged commit d4d78da into master Feb 21, 2023
@onerandomusername onerandomusername deleted the feature/suppress-notifications branch February 21, 2023 01:51
@onerandomusername onerandomusername removed the s: needs review Issue/PR is awaiting reviews label Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: api support Support of Discord API features t: enhancement New feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants