Skip to content

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jun 20, 2016

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-bot nodejs-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

mscdex commented Jun 20, 2016

/cc @bnoordhuis

LGTM

@cjihrig
Copy link
Contributor

cjihrig commented Jun 20, 2016

LGTM

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

LGTM

@jasnell
Copy link
Member

jasnell commented Jun 27, 2016

LGTM

@jasnell
Copy link
Member

jasnell commented Jun 27, 2016

@bnoordhuis ... does this LGTY?

@bnoordhuis bnoordhuis 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

jasnell commented Jun 27, 2016

Landed in 5615487

@jasnell jasnell closed this Jun 27, 2016
@danbev danbev 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
@Fishrock123 Fishrock123 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]>
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]>
@MylesBorins MylesBorins mentioned this pull request Nov 22, 2016
Sign up for free to 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++. dns Issues and PRs related to the dns subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants