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

misc cleanup #5233

Closed
wants to merge 3 commits into from
Closed

misc cleanup #5233

wants to merge 3 commits into from

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Feb 15, 2016

Just a few minor changes.

@mscdex
src: remove unnecessary check
4ad8e5a 
The value's type is unsigned so it will always be >= 0.
@mscdex
http: remove unnecessary check
5875b0c 
The value passed to `process.nextTick()` which is passed to the
callback is already a valid object, so the conditional will always
evaluate to true.
@mscdex
http: remove old, confusing comment
6da8293 
The comment refers to a property (called `_pendings`) that no longer
exists.
@mscdex mscdex added http Issues or PRs related to the http subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. labels Feb 15, 2016
@mscdex
Copy link
Contributor Author

mscdex commented Feb 15, 2016

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

All green except for a few flaky test failures.

@bnoordhuis
Copy link
Member

LGTM

1 similar comment
@jasnell
Copy link
Member

jasnell commented Feb 15, 2016

LGTM

jasnell pushed a commit that referenced this pull request Feb 15, 2016
@mscdex @jasnell
src: remove unnecessary check
b72dbdb 
The value's type is unsigned so it will always be >= 0.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Feb 15, 2016
@mscdex @jasnell
http: remove unnecessary check
0223de6 
The value passed to `process.nextTick()` which is passed to the
callback is already a valid object, so the conditional will always
evaluate to true.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Feb 15, 2016
@mscdex @jasnell
http: remove old, confusing comment
fab240a 
The comment refers to a property (called `_pendings`) that no longer
exists.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Feb 15, 2016

Landed in b72dbdb, 0223de6, and fab240a

@jasnell jasnell closed this Feb 15, 2016
@trevnorris trevnorris mentioned this pull request Feb 15, 2016
Merged
@mscdex mscdex deleted the misc-cleanup branch February 15, 2016 22:17
rvagg pushed a commit that referenced this pull request Feb 15, 2016
@mscdex @rvagg
src: remove unnecessary check
15091cc 
The value's type is unsigned so it will always be >= 0.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this pull request Feb 15, 2016
@mscdex @rvagg
http: remove unnecessary check
ed36235 
The value passed to `process.nextTick()` which is passed to the
callback is already a valid object, so the conditional will always
evaluate to true.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this pull request Feb 15, 2016
@mscdex @rvagg
http: remove old, confusing comment
b706b0c 
The comment refers to a property (called `_pendings`) that no longer
exists.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
stefanmb pushed a commit to stefanmb/node that referenced this pull request Feb 23, 2016
@mscdex @stefanmb
src: remove unnecessary check
8990362 
The value's type is unsigned so it will always be >= 0.

PR-URL: nodejs#5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
stefanmb pushed a commit to stefanmb/node that referenced this pull request Feb 23, 2016
@mscdex @stefanmb
http: remove unnecessary check
0a6c61d 
The value passed to `process.nextTick()` which is passed to the
callback is already a valid object, so the conditional will always
evaluate to true.

PR-URL: nodejs#5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
stefanmb pushed a commit to stefanmb/node that referenced this pull request Feb 23, 2016
@mscdex @stefanmb
http: remove old, confusing comment
cfe2138 
The comment refers to a property (called `_pendings`) that no longer
exists.

PR-URL: nodejs#5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

This PR is not landing cleanly on LTS. Would someone be willing to backport?

@MylesBorins MylesBorins mentioned this pull request Mar 17, 2016
Closed
@MylesBorins
Copy link
Contributor

so it turns out b72dbdb is not landing as we have not yet backported the AsyncWrap changes it modifies.
0223de6, and fab240a land cleanly.

I'm going to move this to land-on for now and have documented b72dbdb in the LTS thread dedicated to tracking AsyncWrap commits that need backporting

MylesBorins pushed a commit that referenced this pull request Mar 17, 2016
@mscdex
http: remove unnecessary check
aaacf7f 
The value passed to `process.nextTick()` which is passed to the
callback is already a valid object, so the conditional will always
evaluate to true.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 17, 2016
@mscdex
http: remove old, confusing comment
20b2dfe 
The comment refers to a property (called `_pendings`) that no longer
exists.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

Adding a dont-land label for the single commit that we are not going to be including until async-wrap

MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
@mscdex
http: remove unnecessary check
ca5d7a8 
The value passed to `process.nextTick()` which is passed to the
callback is already a valid object, so the conditional will always
evaluate to true.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
@mscdex
http: remove old, confusing comment
f982632 
The comment refers to a property (called `_pendings`) that no longer
exists.

PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@AndreasMadsen AndreasMadsen mentioned this pull request May 29, 2016
Closed
3 tasks
MylesBorins pushed a commit that referenced this pull request Jun 7, 2016
@mscdex
src: remove unnecessary check
13487ed 
The value's type is unsigned so it will always be >= 0.

Ref: #7048
PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
@mscdex
src: remove unnecessary check
031b8ce 
The value's type is unsigned so it will always be >= 0.

Ref: #7048
PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 28, 2016
@mscdex
src: remove unnecessary check
7cea165 
The value's type is unsigned so it will always be >= 0.

Ref: #7048
PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
@mscdex
src: remove unnecessary check
68f391b 
The value's type is unsigned so it will always be >= 0.

Ref: #7048
PR-URL: #5233
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
Merged
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
http Issues or PRs related to the http subsystem. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mscdex @bnoordhuis @jasnell @MylesBorins