Skip to content

Conversation

@Trott
Copy link
Member

@Trott Trott commented Jan 29, 2016

A few variables in lib/dgram.js are redeclared with var in a scope
where they have already been declared. These instances can be scoped
more narrowly with const, so that's what this change does.

@Trott Trott added the dgram Issues and PRs related to the dgram subsystem / UDP. label Jan 29, 2016
@Trott
Copy link
Member Author

Trott commented Jan 29, 2016

@jasnell
Copy link
Member

jasnell commented Jan 29, 2016

LGTM
lts-watch label applied

@cjihrig
Copy link
Contributor

cjihrig commented Jan 29, 2016

LGTM

A few variables in `lib/dgram.js` are redeclared with `var` in a scope
where they have already been declared. These instances can be scoped
more narrowly with `const`, so that's what this change does.
@Trott Trott force-pushed the dgram-no-redeclare branch from de3c655 to 727d89b Compare January 29, 2016 19:08
Trott added a commit to Trott/io.js that referenced this pull request Jan 31, 2016
A few variables in `lib/dgram.js` are redeclared with `var` in a scope
where they have already been declared. These instances can be scoped
more narrowly with `const`, so that's what this change does.

PR-URL: nodejs#4940
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member Author

Trott commented Jan 31, 2016

Landed in 1800bf4

@Trott Trott closed this Jan 31, 2016
rvagg pushed a commit that referenced this pull request Feb 10, 2016
A few variables in `lib/dgram.js` are redeclared with `var` in a scope
where they have already been declared. These instances can be scoped
more narrowly with `const`, so that's what this change does.

PR-URL: #4940
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

@Trott this commit relies on a semver minor change 137f53c#diff-5c0ca4f44209cc4cc68c4dccadbd4a07

would you be able to manually back port it?

@Trott
Copy link
Member Author

Trott commented Feb 17, 2016

@thealphanerd It applies cleanly for me against de0e293 on the v4.x-staging branch. Or is the problem not that it doesn't apply cleanly but that it breaks once it's applied?

@MylesBorins
Copy link
Contributor

@Trott have you updated the branch? How are you landing it? git am is blowing up for me

https://gist.github.com/6c9069bd7457291af1cb

@Trott
Copy link
Member Author

Trott commented Feb 17, 2016

It's working for me like this:

$ git fetch upstream && git rebase upstream/v4.x-staging && git checkout -b test-for-myles && curl -L https://github.com/nodejs/node/pull/4940.patch | git am
remote: Counting objects: 5, done.
remote: Compressing objects: 100% (1/1), done.
remote: Total 5 (delta 4), reused 5 (delta 4), pack-reused 0
Unpacking objects: 100% (5/5), done.
From github.com:nodejs/node
   a99111a..0242c87  v4.x-staging -> upstream/v4.x-staging
First, rewinding head to replay your work on top of it...
Fast-forwarded v4.x-staging to upstream/v4.x-staging.
Switched to a new branch 'test-for-myles'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   138    0   138    0     0    269      0 --:--:-- --:--:-- --:--:--   270
  0     0    0  1805    0     0   1776      0 --:--:--  0:00:01 --:--:-- 1762k
Applying: dgram: scope redeclared variables
$ 

Trott added a commit to Trott/io.js that referenced this pull request Feb 18, 2016
A few variables in `lib/dgram.js` are redeclared with `var` in a scope
where they have already been declared. These instances can be scoped
more narrowly with `const`, so that's what this change does.

PR-URL: nodejs#4940
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 18, 2016
A few variables in `lib/dgram.js` are redeclared with `var` in a scope
where they have already been declared. These instances can be scoped
more narrowly with `const`, so that's what this change does.

PR-URL: #4940
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 18, 2016
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
A few variables in `lib/dgram.js` are redeclared with `var` in a scope
where they have already been declared. These instances can be scoped
more narrowly with `const`, so that's what this change does.

PR-URL: #4940
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
A few variables in `lib/dgram.js` are redeclared with `var` in a scope
where they have already been declared. These instances can be scoped
more narrowly with `const`, so that's what this change does.

PR-URL: nodejs#4940
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott Trott deleted the dgram-no-redeclare branch January 13, 2022 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dgram Issues and PRs related to the dgram subsystem / UDP.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants