Skip to content

Conversation

@sinni800
Copy link
Contributor

@sinni800 sinni800 commented Jul 3, 2023

Since changing specific application command permissions is impossible with regular bot tokens anyway, at least we can set the default required permissions to run a command on add.

Copy link
Owner

@jchristgit jchristgit 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 for the pull request!

Two small comments, apart from that it's good to merge 🙂

Comment on lines +272 to +281
@doc """
An optional callback that returns a bitset for the required default permissions to run this command.
Example callback that requires that the user has the permission to ban members to be able to see and execute this command
```elixir
def default_member_permissions, do:
Nostrum.Permission.to_bitset([:ban_members])
```
"""
Copy link
Owner

Choose a reason for hiding this comment

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

Let's dedent this by two spaces so it matches the rest of the module

Suggested change
@doc """
An optional callback that returns a bitset for the required default permissions to run this command.
Example callback that requires that the user has the permission to ban members to be able to see and execute this command
```elixir
def default_member_permissions, do:
Nostrum.Permission.to_bitset([:ban_members])
```
"""
@doc """
An optional callback that returns a bitset for the required default permissions to run this command.
Example callback that requires that the user has the permission to ban members to be able to see and execute this command
```elixir
def default_member_permissions, do:
Nostrum.Permission.to_bitset([:ban_members])

"""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll need a bit more time to do your recommended changes for personal reasons, but it's noted!

Copy link
Owner

Choose a reason for hiding this comment

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

No rush 🙂 If I can help out let me know!

@jchristgit
Copy link
Owner

@sinni800 There has been another pull request opened #30 that adds support for plenty more slash command options, including default_member_permissions as added here.
Do you see a nice way to integrate the PRs with each other?

@sinni800
Copy link
Contributor Author

I think these doesn't really need to be any integration, since the other PR supports what I do anyway

@jchristgit
Copy link
Owner

Thank you for the PR, and sorry for the long wait here. I applied it in master. Have a great weekend!

@jchristgit jchristgit closed this Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants