Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(webhook): Add Webhook Examples #34

Merged
merged 11 commits into from
May 29, 2024

Conversation

davidkathoh
Copy link
Contributor

This pull request adds examples of webhook functions to the project. The new examples demonstrate how to implement and use webhooks effectively, providing clear and practical usage scenarios.

These additions aim to help developers understand and implement webhook functionality more easily.

@0xIchigo 0xIchigo self-requested a review May 29, 2024 03:50
Copy link
Collaborator

@0xIchigo 0xIchigo left a comment

Choose a reason for hiding this comment

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

Very based PR! Just a few comments regarding the main functions having return types and making other types more explicit. Solid content-wise though 🔥

use helius::Helius;

#[tokio::main]
async fn main() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should have a return type. You could import Result from use helius::error::Result and use the type alias for the return type so the line reads async fn main() -> Result<()> {. You'll also have to add in Ok(()) at the end of the block to satisfy the return type. If you get confused, I recommend looking at the other examples we currently have for the SDK


let webhook_id = "your_webhook_id";
let new_addresses = ["your_address1".to_string(), "your_address2".to_string()];
let response = helius.append_addresses_to_webhook(webhook_id, &new_addresses).await;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make the response type explicit here, please?

};

#[tokio::main]
async fn main() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment re main that was made in examples/append_address_to_webhook.rs. All of the example files should follow this convention

encoding: Default::default(),
};

let response = helius.create_webhook(request).await;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you also make this response type explicit, please?

Copy link
Collaborator

Choose a reason for hiding this comment

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

And can you please make all the remaining response vars in the subsequent examples explicit, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be good now

examples/append_address_to_webhook.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@0xIchigo 0xIchigo left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

@0xIchigo 0xIchigo merged commit ed4cb4f into helius-labs:dev May 29, 2024
1 check failed
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