Skip to content
This repository was archived by the owner on Mar 11, 2026. It is now read-only.

add webhook naming via sdk#492

Merged
GregoryElliott merged 2 commits intomasterfrom
gelliott/sdk_naming
Apr 11, 2025
Merged

add webhook naming via sdk#492
GregoryElliott merged 2 commits intomasterfrom
gelliott/sdk_naming

Conversation

@GregoryElliott
Copy link
Copy Markdown
Contributor

@GregoryElliott GregoryElliott commented Apr 10, 2025

add name as an option to the create-webhook api SDK - the name parameter already exists on the create-webhook alchemy api

ASANA TASK: https://app.asana.com/1/1129441638109975/project/1208969177908953/task/1209648386732973?focus=true

@GregoryElliott GregoryElliott marked this pull request as ready for review April 10, 2025 20:35
Comment thread src/api/notify-namespace.ts Outdated
...(rawWebhook.app_id !== undefined && { appId: rawWebhook.app_id })
...(rawWebhook.app_id !== undefined && { appId: rawWebhook.app_id }),
// Only include the name in the final response if it's defined
...(rawWebhook.name !== undefined && { name: rawWebhook.name })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You might not need this extra check as all webhook types can have a name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmm - name is a nullable field - seems like a good idea to keep the check? we did the same thing for app_id which is also nullable

Comment thread src/types/types.ts Outdated
*/
network?: Network;
/** Optional name for the webhook. */
name?: string;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How do you feel about defining a base CommonWebhookParams with name property that all of these interfaces would extend?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i like that - updated

@GregoryElliott GregoryElliott merged commit fc143cb into master Apr 11, 2025
@GregoryElliott GregoryElliott deleted the gelliott/sdk_naming branch April 11, 2025 18:06
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.

2 participants