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: run onResponse on success #177

Merged
merged 2 commits into from
Aug 14, 2024
Merged

fix: run onResponse on success #177

merged 2 commits into from
Aug 14, 2024

Conversation

dawksh
Copy link
Contributor

@dawksh dawksh commented Jun 18, 2024

Motivation

onResponse runs after the return statement hence doesn't run on the success condition.

Change Summary

call onReponse before returning from the function

Merge Checklist

Choose all relevant options below by adding an x now or at any time before submitting for review

  • PR title adheres to the conventional commits standard
  • PR has a changeset
  • PR has been tagged with a change label(s) (i.e. documentation, feature, bugfix, or chore)
  • PR includes documentation if necessary
  • All commits have been signed

Additional Context

If this is a relatively large or complex change, provide more details here that will help reviewers

@horsefacts horsefacts merged commit fb69861 into farcasterxyz:main Aug 14, 2024
0 of 3 checks passed
@horsefacts
Copy link
Collaborator

Thanks for this, @dawksh!

@horsefacts horsefacts mentioned this pull request Aug 14, 2024
5 tasks
horsefacts added a commit that referenced this pull request Aug 14, 2024
## Motivation

#177 had the right idea, but we need to call `onResponse` on every
response, not just success.

## Change Summary

Move `onResponse` outside success code check.

## Merge Checklist

_Choose all relevant options below by adding an `x` now or at any time
before submitting for review_

- [x] PR title adheres to the [conventional
commits](https://www.conventionalcommits.org/en/v1.0.0/) standard
- [ ] PR has a changeset
- [x] PR has been tagged with a change label(s) (i.e. documentation,
feature, bugfix, or chore)
- [x] PR includes documentation if necessary
- [x] All commits have been signed
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