Skip to content

Conversation

Soheab
Copy link
Contributor

@Soheab Soheab commented Jun 25, 2025

Summary

This PR makes it possible to sync single Command/AppCommand instances by adding the following methods:

To AppCommand: sync()
To CommandTree (because Command n co have no state for obvious reasons): .sync_command(<command>, guild=... | None)

This PR also fixes a little bug where upserting an AppCommand could raise a BadRequest when contexts or integration_types were an empty list and .to_dict() still included it.

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, ...)

@Rapptz
Copy link
Owner

Rapptz commented Jun 25, 2025

This endpoint being exposed is pretty dangerous because it's not the proper way to do it leading to potential misunderstanding where you loop through commands and call sync on each. Plus there's no real reason to only sync one command when the bulk one does the same exact thing but better.

@Soheab
Copy link
Contributor Author

Soheab commented Jun 26, 2025

This endpoint being exposed is pretty dangerous because it's not the proper way to do it leading to potential misunderstanding where you loop through commands and call sync on each. Plus there's no real reason to only sync one command when the bulk one does the same exact thing but better.

This is mainly intended for API completion, for those who want to use it over manually calling the HTTP methods. But I get your point about this being risky. I've added a note saying that this isn't the recommended way to handle syncing for that reason, and honestly, there isn't much more we can do to prevent people from calling this when not necessary, and I don't think people would discover this method any more easily than just using sync(), unless a misleading tutorial points them this way.

There's also an additional benefit here, depending on how you look at it, it allows for syncing a command to both a guild and global scope, and discord.py will still handle this correctly because of its fallback to global behaviour.

@Rapptz
Copy link
Owner

Rapptz commented Jun 26, 2025

Once again, I am unsure what the benefit is over using the bulk endpoint. API completion isn't really an argument here since it's just a flat out inferior route.

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