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

src: remove unused parameter #13085

Merged
merged 2 commits into from
May 22, 2017
Merged

src: remove unused parameter #13085

merged 2 commits into from
May 22, 2017

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

@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

Should this be backported to v6.x?

@MylesBorins
Copy link
Contributor

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

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

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

@ronkorving ronkorving ronkorving approved these changes

@jasnell jasnell jasnell approved these changes

@danbev danbev danbev approved these changes

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