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

Properly report error if select() is called without any args #70

Merged
merged 1 commit into from
Sep 18, 2018

Conversation

kasrak
Copy link
Contributor

@kasrak kasrak commented Sep 14, 2018

Fixes the issue reported in this comment: #69 (comment)

Previously, the promise callback would get set as the first arg if select() was called without arguments, which led to confusing behavior, since the promise would reject with the first page of records. Now, it'll throw an error, since select() must be called with at least 1 callback.

@kasrak kasrak requested a review from EvanHahn September 14, 2018 04:28
Copy link
Contributor

@EvanHahn EvanHahn left a comment

Choose a reason for hiding this comment

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

This code looks good but I'd really prefer to add tests because this is subtle behavior. Would you mind adding those?

@kasrak
Copy link
Contributor Author

kasrak commented Sep 18, 2018

Good call, added internal tests 👍

@kasrak kasrak merged commit 98107af into master Sep 18, 2018
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