Skip to content

doc,assert: rename stackStartFunction#22077

Closed
eyqs wants to merge 1 commit into
nodejs:masterfrom
eyqs:patch-1
Closed

doc,assert: rename stackStartFunction#22077
eyqs wants to merge 1 commit into
nodejs:masterfrom
eyqs:patch-1

Conversation

@eyqs

@eyqs eyqs commented Aug 1, 2018

Copy link
Copy Markdown
Contributor

assert.js uses stackStartFn everywhere instead of stackStartFunction. This also increases consistency with stackStartFn in the AssertionError options.

Checklist

[assert.js](https://github.com/nodejs/node/blob/master/lib/assert.js) uses `stackStartFn` everywhere instead of `stackStartFunction`. This also increases consistency with `stackStartFn` in the `AssertionError` options.
@nodejs-github-bot nodejs-github-bot added assert Issues and PRs related to the assert subsystem. doc Issues and PRs related to the documentations. labels Aug 1, 2018

@Trott Trott left a comment

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.

LGTM, although honestly, I'd prefer going to the more verbose stackStartFunction if there's no downside.

@Trott

Trott commented Aug 2, 2018

Copy link
Copy Markdown
Member

Welcome, @eyqs, and thanks for the PR!

@eyqs

eyqs commented Aug 2, 2018

Copy link
Copy Markdown
Contributor Author

I agree that it would be better for everything to use stackStartFunction instead, but it might be a problem for people using stackStartFn in the AssertionError options.

@vsemozhetbyt

Copy link
Copy Markdown
Contributor

@vsemozhetbyt

Copy link
Copy Markdown
Contributor

Node.js Collaborators, please, add 👍 here if you approve fast-tracking.

@vsemozhetbyt vsemozhetbyt added fast-track PRs that do not need to wait for 72 hours to land. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Aug 2, 2018
@vsemozhetbyt

Copy link
Copy Markdown
Contributor

Landed in ce98e2e
Thank you!

vsemozhetbyt pushed a commit that referenced this pull request Aug 2, 2018
[assert.js](https://github.com/nodejs/node/blob/master/lib/assert.js)
uses `stackStartFn` everywhere instead of `stackStartFunction`.

This also increases consistency with `stackStartFn`
in the `AssertionError` options.

PR-URL: #22077
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@eyqs eyqs deleted the patch-1 branch August 2, 2018 14:42
targos pushed a commit that referenced this pull request Aug 2, 2018
[assert.js](https://github.com/nodejs/node/blob/master/lib/assert.js)
uses `stackStartFn` everywhere instead of `stackStartFunction`.

This also increases consistency with `stackStartFn`
in the `AssertionError` options.

PR-URL: #22077
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

assert Issues and PRs related to the assert subsystem. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. fast-track PRs that do not need to wait for 72 hours to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants