FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

docs: Clarify assert.doesNotThrow behavior by foliveira · Pull Request #2807 · nodejs/node · GitHub

/ node Public

docs: Clarify assert.doesNotThrow behavior - #2807

Closed
foliveira wants to merge 1 commit into
nodejs:masterfrom
foliveira:2385-assert-doesNotThrow-docs
ConversationCommits (1)ChecksFiles changed
Closed

docs: Clarify assert.doesNotThrow behavior#2807
foliveira wants to merge 1 commit into
nodejs:masterfrom
foliveira:2385-assert-doesNotThrow-docs

Conversation

foliveira commented Sep 11, 2015

Copy link
Copy Markdown
Contributor

Relates to #2385

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

ChALkeR added doc Issues and PRs related to the documentations. assert Issues and PRs related to the assert subsystem. labels Sep 11, 2015
Trott reviewed Sep 11, 2015
View reviewed changes
Comment thread doc/api/assert.markdown Outdated

Trott Sep 11, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

foliveira commented Sep 11, 2015

Copy link
Copy Markdown
Contributor Author

@Trott updated the branch with your suggestions

Trott reviewed Sep 11, 2015
View reviewed changes
Comment thread doc/api/assert.markdown Outdated

Trott Sep 11, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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().

foliveira mentioned this pull request Sep 11, 2015
Closed
Trott reviewed Sep 11, 2015
View reviewed changes
Comment thread doc/api/assert.markdown Outdated

Trott Sep 11, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Trott commented Sep 11, 2015

Copy link
Copy Markdown
Member

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

foliveira commented Sep 11, 2015

Copy link
Copy Markdown
Contributor Author

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

Trott commented Sep 11, 2015

Copy link
Copy Markdown
Member

👍

LGTM

docs: Clarify assert.doesNotThrow behavior
e8bb02f 
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.
foliveira force-pushed the 2385-assert-doesNotThrow-docs branch from e7e4c1a to e8bb02f Compare September 14, 2015 13:14

foliveira commented Sep 14, 2015

Copy link
Copy Markdown
Contributor Author

Pushed rebased commits

Trott commented Sep 14, 2015

Copy link
Copy Markdown
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.

silverwind mentioned this pull request Sep 23, 2015
Closed
Trott pushed a commit that referenced this pull request Sep 23, 2015
docs: Clarify assert.doesNotThrow behavior
79ebeab 
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 <rtrott@gmail.com>

Trott commented Sep 23, 2015

Copy link
Copy Markdown
Member

Landed in 79ebeab. Thanks!

Trott closed this Sep 23, 2015
Fishrock123 pushed a commit that referenced this pull request Sep 25, 2015
docs: Clarify assert.doesNotThrow behavior
b28f6a5 
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 <rtrott@gmail.com>
This was referenced Sep 30, 2015
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

assert Issues and PRs related to the assert subsystem. doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL