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

buffer: remove obsolete and confusing comment #7264

Closed
wants to merge 1 commit into from
Closed

buffer: remove obsolete and confusing comment #7264

wants to merge 1 commit into from

Conversation

seishun
Copy link
Contributor

@seishun seishun commented Jun 10, 2016

Checklist
Affected core subsystem(s)

buffer

Description of change

This comment applied to a line that was removed in dd67608 and is no longer relevant.

...Or so it seems. The reason for keeping it was possibly discussed in https://github.com/nodejs/node-private/pull/30, but I don't have access to it.

This comment applied to a line that was removed in
dd67608
and is no longer relevant.
@nodejs-github-bot nodejs-github-bot added the buffer Issues and PRs related to the buffer subsystem. label Jun 10, 2016
@mscdex
Copy link
Contributor

mscdex commented Jun 10, 2016

/cc @nodejs/buffer

@bnoordhuis
Copy link
Member

LGTM. It's indeed a leftover from refactoring.

@addaleax
Copy link
Member

LGTM

seishun added a commit that referenced this pull request Jun 12, 2016
This comment applied to a line that was removed in
dd67608
and is no longer relevant.

PR-URL: #7264
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@seishun
Copy link
Contributor Author

seishun commented Jun 12, 2016

Landed in bd23290.

@seishun seishun closed this Jun 12, 2016
@ChALkeR
Copy link
Member

ChALkeR commented Jun 12, 2016

Yes, that comment was irrelevant after the change, I forgot to remove it in my commit.
Thanks!

@evanlucas
Copy link
Contributor

This is not landing cleanly on v6.x. Going to hold off for now

@ChALkeR
Copy link
Member

ChALkeR commented Jun 16, 2016

@evanlucas Strange. It should land cleanly, it's just a removal of lines https://github.com/nodejs/node/blob/v6.x/lib/buffer.js#L213-L215.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants