Skip to content

src: remove unused parameter#13085

Merged
mscdex merged 2 commits intonodejs:masterfrom
mscdex:src-remove-unused-param
May 22, 2017
Merged

src: remove unused parameter#13085
mscdex merged 2 commits intonodejs:masterfrom
mscdex:src-remove-unused-param

Conversation

@mscdex
Copy link
Contributor

@mscdex mscdex commented May 17, 2017 •
edited
Loading

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

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

@mscdex mscdex added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 17, 2017
@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels May 17, 2017
@mscdex mscdex force-pushed the src-remove-unused-param branch from a596665 to c54fbd1 Compare May 18, 2017 03:20
@mscdex
Copy link
Contributor Author

mscdex commented May 18, 2017

I've removed some more unused parameters and fixed a code typo (although for some reason it didn't seem to make any difference :-S)

@mscdex mscdex added the semver-major PRs that contain breaking changes and should be released in the next major version. label May 18, 2017
@mscdex
Copy link
Contributor Author

mscdex commented May 18, 2017

I'm marking this as semver-major as it probably should be? If not, feel free to remove the label.

@addaleax
Copy link
Member

addaleax commented May 18, 2017

@mscdex I don’t think you’re touching anything not guarded by NODE_WANT_INTERNALS, right?

@addaleax addaleax removed the semver-major PRs that contain breaking changes and should be released in the next major version. label May 18, 2017
@mscdex
Copy link
Contributor Author

mscdex commented May 18, 2017

@addaleax Ah you're probably right about that. Thanks.

mscdex added 2 commits May 22, 2017 16:09
@mscdex
src: remove unused parameters
f529706 
PR-URL: nodejs#13085
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@mscdex
src: fix typo
dcc59f9 
PR-URL: nodejs#13085
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@mscdex mscdex force-pushed the src-remove-unused-param branch from c54fbd1 to dcc59f9 Compare May 22, 2017 20:09
@mscdex mscdex merged commit dcc59f9 into nodejs:master May 22, 2017
@mscdex mscdex deleted the src-remove-unused-param branch May 22, 2017 20:12
jasnell pushed a commit that referenced this pull request May 23, 2017
@mscdex @jasnell
src: remove unused parameters
116ea39 
PR-URL: #13085
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
jasnell pushed a commit that referenced this pull request May 23, 2017
@mscdex @jasnell
src: fix typo
7c4401b 
PR-URL: #13085
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
jasnell pushed a commit that referenced this pull request May 23, 2017
@mscdex @jasnell
src: remove unused parameters
1263b70 
PR-URL: #13085
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
jasnell pushed a commit that referenced this pull request May 23, 2017
@mscdex @jasnell
src: fix typo
85e2d56 
PR-URL: #13085
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@jasnell jasnell mentioned this pull request May 28, 2017
Closed
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
@MylesBorins
Copy link
Contributor

MylesBorins commented Jul 17, 2017

Should this be backported to v6.x?

@MylesBorins
Copy link
Contributor

MylesBorins commented Aug 14, 2017

ping @mscdex

@mscdex
Copy link
Contributor Author

mscdex commented Aug 14, 2017

I don't know, but I won't have time to backport anytime soon.

@MylesBorins
Copy link
Contributor

MylesBorins commented Sep 19, 2017

setting as don't land we can revisit later if we find there are issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

+2 more reviewers

@danbev danbev danbev approved these changes

@ronkorving ronkorving ronkorving approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@mscdex @addaleax @MylesBorins @danbev @jasnell @ronkorving @cjihrig @nodejs-github-bot