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

lib: Remove unnecessary TODO comments #4719

Closed
wants to merge 1 commit into from

Conversation

pgeiss
Copy link
Contributor

@pgeiss pgeiss commented Jan 16, 2016

Original committer (@trevnorris) stated they were safe to remove in issue comments.
Ref: Issue #4642

PS: I want to work more on the TODOs in 4642 (and, you know, actually fix things). I'm assuming I should direct questions if I have any there? It has a 'mentor-available' tag.

@mscdex mscdex added the buffer Issues and PRs related to the buffer subsystem. label Jan 16, 2016
@Trott
Copy link
Member

Trott commented Jan 16, 2016

LGTM

The mentor who is available is probably @Fishrock123. (I'm inferring this from the fact that he's the one that added the mentor-available tag.) But yes, directing questions about #4642 to that actual issue is definitely the thing to do. I'm happy to help too, at least on areas of the code where I feel comfortable. And there are probably others too. If you want to try as many venues as possible, you can also try asking in the #node-dev IRC channel.

Original committer stated they were safe to remove in issue post.
Ref: Issue nodejs#4642
@pgeiss
Copy link
Contributor Author

pgeiss commented Jan 16, 2016

Rebased on latest master.

cjihrig pushed a commit that referenced this pull request Jan 17, 2016
Refs: #4642
PR-URL: #4719
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@cjihrig
Copy link
Contributor

cjihrig commented Jan 17, 2016

Thanks! Landed in 83d2b77. A full CI run isn't necessary for removing a few comments, but I did run the tests locally as a sanity check.

@cjihrig cjihrig closed this Jan 17, 2016
@rvagg
Copy link
Member

rvagg commented Jan 18, 2016

Thanks for finding a place to contribute @pgeiss, it looks like this is your first commit in core, if so, welcome on board! I hope you can find other places to make an impact.

evanlucas pushed a commit that referenced this pull request Jan 18, 2016
Refs: #4642
PR-URL: #4719
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 28, 2016
Refs: #4642
PR-URL: #4719
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
Refs: #4642
PR-URL: #4719
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
Refs: nodejs#4642
PR-URL: nodejs#4719
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 11, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 13, 2016
Refs: nodejs#4642
PR-URL: nodejs#4719
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
Refs: nodejs#4642
PR-URL: nodejs#4719
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Refs: nodejs#4642
PR-URL: nodejs#4719
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
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.

7 participants