Skip to content
This repository has been archived by the owner on Mar 6, 2022. It is now read-only.

Make application commands register on login #39

Closed
wants to merge 2 commits into from
Closed

Make application commands register on login #39

wants to merge 2 commits into from

Conversation

Hype3808
Copy link
Contributor

@Hype3808 Hype3808 commented Jan 1, 2022

Summary

Make application commands register on login instead of on_connect event so you don't have to manually register the slash commands when u call a on_connect.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • 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, ...)

Extra Checklist

  • No errors have been raised when starting the bot

Confirmations

  • I have checked the open PRs and no PR exists like this one.
  • My PR focuses on only one issue/change.

Copy link
Contributor

@VincentRPS VincentRPS left a comment

Choose a reason for hiding this comment

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

lgtm, but if this is on_connect why not put it in connect?

@Hype3808
Copy link
Contributor Author

Hype3808 commented Jan 2, 2022

lgtm, but if this is on_connect why not put it in connect?

There is too much stuffs in connect method. It will be better to put it in login as we want application commands to register when logging in.

@izxxr
Copy link
Collaborator

izxxr commented Jan 4, 2022

On second thought, there should be a separate function to do this that get's called in start()

@Hype3808
Copy link
Contributor Author

Hype3808 commented Jan 4, 2022

On second thought, there should be a separate function to do this that get's called in start()

Do you mean that changing it from login to start?

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants