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.

Sign up for GitHub

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

Jump to bottom

assert: name anonymous functions #9051

Closed
wants to merge 1 commit into from
Closed

assert: name anonymous functions #9051

wants to merge 1 commit into from

Conversation

maasencioh
Copy link
Contributor

@maasencioh maasencioh commented Oct 12, 2016 •
edited
Loading

Checklist
  • commit message follows commit guidelines
Description of change

Ref: #8913

@nodejs-github-bot nodejs-github-bot added the assert Issues and PRs related to the assert subsystem. label Oct 12, 2016
lpinca
lpinca approved these changes Oct 12, 2016
View reviewed changes
Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM

lpinca
lpinca reviewed Oct 14, 2016
View reviewed changes
lib/assert.js
_throws(true, block, error, message);
};

// EXTENSION! This is annoying to write outside this module.
assert.doesNotThrow = function(block, /*optional*/error, /*optional*/message) {
assert.doesNotThrow = function doesNotThrow(block, /*optional*/error, /*optional*/message) {
Copy link
Member

Choose a reason for hiding this comment

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

@maasencioh I think the linter is not happy with this line as it is too long.
Can you please fix this? Thank you.

@maasencioh
Copy link
Contributor Author

@lpinca done!

@lpinca
Copy link
Member

lpinca commented Oct 17, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/4538/

@rvagg rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02
@jasnell jasnell self-assigned this Oct 19, 2016
jasnell pushed a commit that referenced this pull request Oct 19, 2016
@maasencioh @jasnell
assert: name anonymous functions
ef030da 
Ref: #8913
PR-URL: #9051
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Oct 19, 2016

landed in ef030da. Thank you!

@jasnell jasnell closed this Oct 19, 2016
@maasencioh
Copy link
Contributor Author

Thanks!

@maasencioh maasencioh deleted the name_assert branch October 19, 2016 16:06
jasnell pushed a commit that referenced this pull request Oct 20, 2016
@maasencioh @jasnell
assert: name anonymous functions
1043f5d 
Ref: #8913
PR-URL: #9051
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 17, 2016
Closed
@MylesBorins MylesBorins added the baking-for-lts PRs that need to wait before landing in a LTS release. label May 9, 2017
@MylesBorins MylesBorins added dont-land-on-v6.x and removed baking-for-lts PRs that need to wait before landing in a LTS release. lts-watch-v6.x labels Nov 14, 2017
@aliscco aliscco mentioned this pull request Dec 1, 2023
Open
@leonardoadame leonardoadame mentioned this pull request Dec 3, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

Assignees

@jasnell jasnell

Labels
assert Issues and PRs related to the assert subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@maasencioh @lpinca @jasnell @MylesBorins @nodejs-github-bot