Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve/refactor .cancel() #309

Merged
merged 4 commits into from
Jun 20, 2019
Merged

Improve/refactor .cancel() #309

merged 4 commits into from
Jun 20, 2019

Conversation

ehmicky
Copy link
Collaborator

@ehmicky ehmicky commented Jun 20, 2019

This separates cancel() into its own function.

Also this makes cancel() return a boolean like kill() does. The fact that kill() returns a boolean is not documented and is not part of @types/node so I think we should do the same, and not document it nor type it. However it would not hurt to still forward kill() return value.

@sindresorhus
Copy link
Owner

The fact that kill() returns a boolean is not documented

You should open a Node.js issue about either documenting it or removing that return value.

However it would not hurt to still forward kill() return value.

I disagree. It breaks the least amount of surprise rule. We should not do undocumented stuff. I think we should not return a boolean. If anyone needs it, they will request it and we can consider adding it officially.


So I don't think cancel should return anything either, at least for now.

@ehmicky
Copy link
Collaborator Author

ehmicky commented Jun 20, 2019

Fixed.

@sindresorhus sindresorhus merged commit 8af8c96 into master Jun 20, 2019
@sindresorhus sindresorhus deleted the feature/improve-cancel branch June 20, 2019 13:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants