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

lib: move deprecationWarned var #14769

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Aug 11, 2017

The variable deprecationWarned currently looks a little misplaced. It is
used in emitWarning but declared after it. This commit suggest moving
the var to the beginning of the function.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

lib

The variable deprecationWarned currently looks a little misplaced. It is
used in emitWarning but declared after it. This commit suggest moving
the var to the beginning of the function.
@nodejs-github-bot nodejs-github-bot added the process Issues and PRs related to the process subsystem. label Aug 11, 2017
@@ -14,6 +14,8 @@ function getAsynchronousRejectionWarningObject(uid) {
}

function setupPromises(scheduleMicrotasks) {
var deprecationWarned = false;
Copy link
Contributor

@aqrln aqrln Aug 11, 2017

Choose a reason for hiding this comment

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

IMO, it would be nice to change it to let while you are here, but LGTM either way.

Copy link
Member

Choose a reason for hiding this comment

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

Unless it triggers deopts of course 😁

danbev added a commit to danbev/node that referenced this pull request Aug 14, 2017
The variable deprecationWarned currently looks a little misplaced. It is
used in emitWarning but declared after it. This commit suggest moving
the var to the beginning of the function.

PR-URL: nodejs#14769
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danbev
Copy link
Contributor Author

danbev commented Aug 14, 2017

Landed in 170ff0e

@danbev danbev closed this Aug 14, 2017
@danbev danbev deleted the move-deprecationWarned-var branch August 14, 2017 05:46
addaleax pushed a commit that referenced this pull request Aug 14, 2017
The variable deprecationWarned currently looks a little misplaced. It is
used in emitWarning but declared after it. This commit suggest moving
the var to the beginning of the function.

PR-URL: #14769
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax addaleax mentioned this pull request Aug 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants