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

Clarify doesNotThrow parameters #450

Merged
merged 1 commit into from
Nov 25, 2018
Merged

Conversation

axelpale
Copy link
Contributor

To prevent issues like #334, here is a proposal to clarify the purpose of the second argument. My own first reaction to the second argument was like "Whaaat, the docs cannot be right. Even the mere word 'expected' is against the purpose of the assertion. How we can expect something that is not thrown. The argument cannot have any meaning." Then I browsed through the issues, found #334 and then it became clear.

To prevent issues like tape-testing#334, here is a proposal to clarify the purpose of the second argument. My own first reaction to the second argument was like "Whaaat, the docs cannot be right. Even the mere word 'expected' is against the purpose of the assertion. How we can expect something that is not thrown. The argument cannot have any meaning." Then I browsed through the issues, found tape-testing#334 and then it became clear.
Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks, the clarification is appreciated.

@ljharb ljharb merged commit 1225d01 into tape-testing:master Nov 25, 2018
ljharb added a commit that referenced this pull request Dec 29, 2018
 - [Fix] notEqual and notDeepEqual show "expected" value on failure (#454)
 - [Docs] Updated readme to make test, test.only, and test.skip consistent (#452)
 - [Docs] Clarify doesNotThrow parameters (#450)
 - [Docs] Adding tap-junit (#449)
 - [readme] Change broken image to use web archive (#443)
 - [Docs] convert list of tap reporters to links (#439, #440)
 - [Dev Deps] update `eslint`, `eclint`
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.

None yet

2 participants