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: done callback execution when each method limit reached #716

Merged

Conversation

Hunga1
Copy link
Contributor

@Hunga1 Hunga1 commented Dec 17, 2021

Fixes #678

For resources that contain an each method to stream records from the API until a limit is reached, its opts.done callback fails to execute when the user defines a limit in the opts object and it is reached.

Changes include new tests for testing that:

  • The opts.done callback is executed when incomingPhoneNumber records processed
  • The opts.done callback is executed when a user defined limit is reached for incomingPhoneNumber records

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

@Hunga1 Hunga1 changed the title fix: done callback execution when each method limit reached WIP fix: done callback execution when each method limit reached Jan 4, 2022
@Hunga1 Hunga1 force-pushed the fix-done-callback-execution-when-limit-reached branch 2 times, most recently from 76a192e to a9780b9 Compare January 4, 2022 18:00
@Hunga1 Hunga1 changed the title WIP fix: done callback execution when each method limit reached fix: done callback execution when each method limit reached Jan 4, 2022
@Hunga1
Copy link
Contributor Author

Hunga1 commented Jan 4, 2022

I've went ahead and made the changes to address the requests. PR is ready for another look.

@Hunga1 Hunga1 force-pushed the fix-done-callback-execution-when-limit-reached branch from a9780b9 to f27fb59 Compare January 5, 2022 21:19
@Hunga1
Copy link
Contributor Author

Hunga1 commented Jan 5, 2022

PR is ready for another review.

Copy link
Contributor

@childish-sambino childish-sambino left a comment

Choose a reason for hiding this comment

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

LGTM

nit: recommend against force-pushing once a PR has been open. It makes it difficult to track what's changed since the last review.

@Hunga1 Hunga1 merged commit 8314b1b into twilio:main Jan 6, 2022
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.

incomingPhoneNumber.each({ limit, done }, callback) does not call done
2 participants