Skip to content

Conversation

@foliveira
Copy link
Contributor

Relates to #2385

The documentation now seems to be up to date with the code and tests for the assert module

@ChALkeRChALkeR added doc Issues and PRs related to the documentations. assert Issues and PRs related to the assert subsystem. labels Sep 11, 2015
Copy link
Member

Choose a reason for hiding this comment

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

While we're at it, can we fix the comma splice here? This should be two sentences. ...not to throw an error. See...

@foliveira
Copy link
ContributorAuthor

@Trott updated the branch with your suggestions

Copy link
Member

Choose a reason for hiding this comment

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

Two more things (sorry!):

Should probably be assert.throws() with the parentheses, and that text should probably link to the section of the document for assert.throws().

Copy link
Member

Choose a reason for hiding this comment

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

I think this might be clearer if you stayed away from AssertionError. I'd go with maybe SyntaxError.

@Trott
Copy link
Member

I made several suggested changes with inline comments, but LGTM. /cc @nodejs/documentation

@foliveira
Copy link
ContributorAuthor

@Trott thanks for the tips/suggestions. All should be fixed now.

@Trott
Copy link
Member

👍

LGTM

The documentation for assert.doesNotThrow now reflects all the inputs the function accepts, as well as the errors thrown for each combination of parameter types.
@foliveirafoliveiraforce-pushed the 2385-assert-doesNotThrow-docs branch from e7e4c1a to e8bb02fCompareSeptember 14, 2015 13:14
@foliveira
Copy link
ContributorAuthor

Pushed rebased commits

@Trott
Copy link
Member

LGTM. It would be good if someone from @nodejs/documentation could look at it just to make sure it conforms with any emerging or established documentation guidelines that I may be unaware of.

Trott pushed a commit that referenced this pull request Sep 23, 2015
The documentation for assert.doesNotThrow now reflects all the inputs the function accepts, as well as the errors thrown for each combination of parameter types. PR-URL: #2807 Reviewed-By: Rich Trott <[email protected]>
@Trott
Copy link
Member

Landed in 79ebeab. Thanks!

@TrottTrott closed this Sep 23, 2015
Fishrock123 pushed a commit that referenced this pull request Sep 25, 2015
The documentation for assert.doesNotThrow now reflects all the inputs the function accepts, as well as the errors thrown for each combination of parameter types. PR-URL: #2807 Reviewed-By: Rich Trott <[email protected]>
This was referenced Sep 30, 2015
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

assertIssues and PRs related to the assert subsystem.docIssues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@foliveira@Trott@ChALkeR