Skip to content

Conversation

@danbev
Copy link
Contributor

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

src

Description of change

This commit attempts to fix one of the items in
#4641, which was to remove a TODO
comment from env.h regarding the naming of the ares_task_t struct.

Also, the struct ares_task_list was renamed to node_ares_task_list
following the same reasoning that is does not belong to the c-ares API.

This commit attempts to fix one of the items in nodejs#4641, which was to remove a TODO comment from env.h regarding the naming of the ares_task_t struct. Also, the struct ares_task_list was renamed to node_ares_task_list following the same reasoning that is does not belong to the c-ares API.
@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 20, 2016
@MylesBorins
Copy link
Contributor

@mscdex
Copy link
Contributor

/cc @bnoordhuis

LGTM

@cjihrig
Copy link
Contributor

LGTM

@mscdexmscdex added the dns Issues and PRs related to the dns subsystem. label Jun 21, 2016
@addaleax
Copy link
Member

LGTM

@jasnell
Copy link
Member

LGTM

@jasnell
Copy link
Member

@bnoordhuis ... does this LGTY?

@bnoordhuisbnoordhuis removed their assignment Jun 27, 2016
@bnoordhuis
Copy link
Member

LGTM

jasnell pushed a commit that referenced this pull request Jun 27, 2016
This commit attempts to fix one of the items in #4641, which was to remove a TODO comment from env.h regarding the naming of the ares_task_t struct. Also, the struct ares_task_list was renamed to node_ares_task_list following the same reasoning that is does not belong to the c-ares API. PR-URL: #7345 Reviewed-By: Brian White <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
@jasnell
Copy link
Member

Landed in 5615487

@jasnelljasnell closed this Jun 27, 2016
@danbevdanbev deleted the rename-ares-task-struct branch June 30, 2016 16:12
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
This commit attempts to fix one of the items in #4641, which was to remove a TODO comment from env.h regarding the naming of the ares_task_t struct. Also, the struct ares_task_list was renamed to node_ares_task_list following the same reasoning that is does not belong to the c-ares API. PR-URL: #7345 Reviewed-By: Brian White <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Conflicts: src/env.h
@Fishrock123Fishrock123 mentioned this pull request Jul 5, 2016
@MylesBorins
Copy link
Contributor

@addaleax@bnoordhuis lts?

@addaleax
Copy link
Member

@thealphanerd Yes… if it doesn’t land cleanly I can backport.

@MylesBorins
Copy link
Contributor

@addaleax it will need a manual backport thank you. 😄

@addaleax
Copy link
Member

Backport is in #7669

addaleax pushed a commit to addaleax/node that referenced this pull request Jul 12, 2016
This commit attempts to fix one of the items in nodejs#4641, which was to remove a TODO comment from env.h regarding the naming of the ares_task_t struct. Also, the struct ares_task_list was renamed to node_ares_task_list following the same reasoning that is does not belong to the c-ares API. PR-URL: nodejs#7345 Reviewed-By: Brian White <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
addaleax pushed a commit to addaleax/node that referenced this pull request Nov 22, 2016
This commit attempts to fix one of the items in nodejs#4641, which was to remove a TODO comment from env.h regarding the naming of the ares_task_t struct. Also, the struct ares_task_list was renamed to node_ares_task_list following the same reasoning that is does not belong to the c-ares API. PR-URL: nodejs#7345 Reviewed-By: Brian White <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
@addaleaxaddaleax mentioned this pull request Nov 22, 2016
MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
This commit attempts to fix one of the items in #4641, which was to remove a TODO comment from env.h regarding the naming of the ares_task_t struct. Also, the struct ares_task_list was renamed to node_ares_task_list following the same reasoning that is does not belong to the c-ares API. PR-URL: #7345 Reviewed-By: Brian White <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Nov 22, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

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