Skip to content

Conversation

@Zoddo
Copy link
Contributor

@Zoddo Zoddo commented May 31, 2025

I've checked all create/update/delete endpoints, they should all be covered now.

public async deleteWebhook(webhookId: string, token?: string): Promise<RESTDeleteAPIWebhookResult> {
if (token) return this.requestHandler.request(Endpoints.WEBHOOK_TOKEN(webhookId, token), {}, "delete", "json") as RESTDeleteAPIWebhookResult;
return this.requestHandler.request(Endpoints.WEBHOOK(webhookId), {}, "delete", "json") as RESTDeleteAPIWebhookResult;
public async deleteWebhook(webhookId: string, token?: string, reason?: string): Promise<RESTDeleteAPIWebhookResult> {
Copy link
Contributor Author

@Zoddo Zoddo Jun 1, 2025

Choose a reason for hiding this comment

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

Taking a look at this again after sleeping, I'm not sure that's the best way to handle it here. I added it as a second optional argument to not make a breaking change.

However, you may want to set the reason without setting the token. It's technically possible by passing undefined for the token, but it looks a bit weird.

From a lib API perspective, I was wondering if something like that may be cleaner:

public async deleteWebhook(webhookId: string, token?: string): Promise<RESTDeleteAPIWebhookResult>;
public async deleteWebhook(webhookId: string, data?: {token?: string, reason?: string}): Promise<RESTDeleteAPIWebhookResult>;

That way it's not a breaking change either, and we can support a data object with optional properties (instead of relying to optional method parameters that you may have to set to undefined).

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.

1 participant