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

Fix streaming example #205

Merged
merged 4 commits into from
Feb 16, 2024
Merged

Fix streaming example #205

merged 4 commits into from
Feb 16, 2024

Conversation

dkundel
Copy link
Contributor

@dkundel dkundel commented Feb 16, 2024

The example to use replicate.stream was wrongly checking the event type and was not producing any output. I adjusted the example so that it works. The event && in front of the event.event === 'output' check seems unnecessary as I can't think of a scenario where event would be false-y and even event.event seems to always be of type string but I left it for now in case I'm missing something.

The example to use replicate.stream was wrongly checking the event type and was not producing any output.
@mattt
Copy link
Member

mattt commented Feb 16, 2024

Related to #197

Copy link
Member

@mattt mattt left a comment

Choose a reason for hiding this comment

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

Thanks for opening a PR, @dkundel!

I'm happy to merge this in as-is. But for your consideration, I made some suggestions to use destructuring to avoid the event.event awkwardness. WDYT?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
dkundel and others added 3 commits February 16, 2024 13:59
Co-authored-by: Mattt <mattt@me.com>
Co-authored-by: Mattt <mattt@me.com>
Removing the console.dir since it's no longer relevant
@dkundel
Copy link
Contributor Author

dkundel commented Feb 16, 2024

Looks good to me! I did remove the console.dir({event}) since it no longer felt relevant.

@mattt
Copy link
Member

mattt commented Feb 16, 2024

Excellent. Thanks again for your help with this, @dkundel! Merging now.

@mattt mattt merged commit d09067c into replicate:main Feb 16, 2024
2 of 8 checks passed
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.

None yet

2 participants