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

dns: tweak regex for IPv6 addresses #8665

Closed
wants to merge 1 commit into from
Closed

dns: tweak regex for IPv6 addresses #8665

wants to merge 1 commit into from

Conversation

lpinca
Copy link
Member

@lpinca lpinca commented Sep 20, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

dns

Description of change

The regex used in dns.setServers() to match IPv6 addresses in square brackets uses a capturing group for the port but this info is not needed.

This commit replaces the capturing group with a non capturing one.

@lpinca
dns: tweak regex for IPv6 addresses
4c463d2 
The regex used in `dns.setServers()` to match IPv6 addresses in square
brackets uses a capturing group for the port but this info is not
needed.

This commit replaces the capturing group with a non capturing one.
@nodejs-github-bot nodejs-github-bot added the dns Issues and PRs related to the dns subsystem. label Sep 20, 2016
@lpinca
Copy link
Member Author

lpinca commented Sep 20, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/4148/

addaleax
addaleax approved these changes Sep 20, 2016
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 😄

benjamingr
benjamingr approved these changes Sep 20, 2016
View reviewed changes
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice find.

imyller
imyller approved these changes Sep 20, 2016
View reviewed changes
Copy link
Member

@imyller imyller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM good catch

jasnell
jasnell approved these changes Sep 20, 2016
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but we really ought to get away from using a regex for this.

princejwesley
princejwesley approved these changes Sep 20, 2016
View reviewed changes
Copy link
Contributor

@princejwesley princejwesley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

targos
targos approved these changes Sep 20, 2016
View reviewed changes
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@imyller imyller self-assigned this Sep 22, 2016
@imyller
Copy link
Member

imyller commented Sep 22, 2016

I'll start landing this:

  • Six LGTMs
  • No objections
  • CI passed (only the usual CI failures)

imyller pushed a commit to imyller/node that referenced this pull request Sep 22, 2016
@lpinca @imyller
dns: tweak regex for IPv6 addresses
575077a 
The regex used in `dns.setServers()` to match IPv6 addresses in square
brackets uses a capturing group for the port but this info is not
needed.

This commit replaces the capturing group with a non capturing one.

PR-URL: nodejs#8665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@imyller
Copy link
Member

imyller commented Sep 22, 2016

landed in 575077a

@imyller imyller closed this Sep 22, 2016
@imyller imyller removed their assignment Sep 22, 2016
@lpinca lpinca deleted the use/non-capturing-group branch September 23, 2016 06:55
jasnell pushed a commit that referenced this pull request Sep 29, 2016
@lpinca @jasnell
dns: tweak regex for IPv6 addresses
9d4b1f0 
The regex used in `dns.setServers()` to match IPv6 addresses in square
brackets uses a capturing group for the port but this info is not
needed.

This commit replaces the capturing group with a non capturing one.

PR-URL: #8665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@MylesBorins
Copy link
Contributor

adding to lts watch, let me know if it should be removed. This will need a manual backport but I can handle it

@lpinca lpinca mentioned this pull request Oct 7, 2016
Closed
lpinca added a commit to lpinca/node that referenced this pull request Oct 7, 2016
@lpinca
dns: tweak regex for IPv6 addresses
f1ffe35 
The regex used in `dns.setServers()` to match IPv6 addresses in square
brackets uses a capturing group for the port but this info is not
needed.

This commit replaces the capturing group with a non capturing one.

Refs: nodejs#8965
PR-URL: nodejs#8665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
@lpinca
dns: tweak regex for IPv6 addresses
1a54130 
The regex used in `dns.setServers()` to match IPv6 addresses in square
brackets uses a capturing group for the port but this info is not
needed.

This commit replaces the capturing group with a non capturing one.

Ref: #8965
PR-URL: #8665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
@lpinca
dns: tweak regex for IPv6 addresses
e025805 
The regex used in `dns.setServers()` to match IPv6 addresses in square
brackets uses a capturing group for the port but this info is not
needed.

This commit replaces the capturing group with a non capturing one.

Ref: #8965
PR-URL: #8665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
@lpinca @Fishrock123
dns: tweak regex for IPv6 addresses
7875589 
The regex used in `dns.setServers()` to match IPv6 addresses in square
brackets uses a capturing group for the port but this info is not
needed.

This commit replaces the capturing group with a non capturing one.

PR-URL: #8665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
@lpinca @rvagg
dns: tweak regex for IPv6 addresses
46b94cc 
The regex used in `dns.setServers()` to match IPv6 addresses in square
brackets uses a capturing group for the port but this info is not
needed.

This commit replaces the capturing group with a non capturing one.

Ref: #8965
PR-URL: #8665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
@lpinca @MylesBorins
dns: tweak regex for IPv6 addresses
0b87b1a 
The regex used in `dns.setServers()` to match IPv6 addresses in square
brackets uses a capturing group for the port but this info is not
needed.

This commit replaces the capturing group with a non capturing one.

Ref: #8965
PR-URL: #8665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos approved these changes

@imyller imyller imyller approved these changes

@princejwesley princejwesley princejwesley approved these changes

@benjamingr benjamingr benjamingr approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
dns Issues and PRs related to the dns subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@lpinca @imyller @MylesBorins @jasnell @addaleax @benjamingr @princejwesley @targos @nodejs-github-bot