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

Fixed reserveWithTimeout & made connect options optional #10

Closed
wants to merge 4 commits into from
Closed

Fixed reserveWithTimeout & made connect options optional #10

wants to merge 4 commits into from

Conversation

only-cliches
Copy link

According the README connect options are optional:

await beanstalkd.connect() // Connects to localhost:11300
await beanstalkd.connect({ host, port })

The current typescript definitions require the connect options:
alt text

This pull request makes the connect options optional.

@only-cliches
Copy link
Author

reserveWithTimeout command was not working as expected, it would hang indefinitely when I tested it.

The additions to the PR fix the behavior of reserveWithTimeout to make it return an error on timeout and a job on success and also adds reserveWithTimeout to the typescript definition file.

@only-cliches only-cliches changed the title Made connect options optional per API spec Fixed reserveWithTimeout & made connect options optional Aug 28, 2019
@divmgl
Copy link
Member

divmgl commented Dec 4, 2019

Hello @ClickSimply! So it looks like you're solving two separate issues in one PR.

I would recommend opening a separate pull request for the Typescript fix given that it's purely semantic in nature, and having this pull request deal only with reserveWithTimeout. That way I can review reserveWithTimeout on its own.

Also, it looks like you'll need to fix your author email on your Git commits.

Thanks for your help with jackd!

@divmgl divmgl self-assigned this Dec 4, 2019
@divmgl divmgl added the bug Something isn't working label Dec 4, 2019
@divmgl
Copy link
Member

divmgl commented Mar 9, 2021

It looks like this has been resolved for the most part by other PRs addressing TypeScript fixes and this PR: #14

@divmgl divmgl closed this Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants