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

nsqadmin: Fix handling of IPv6 broadcast addresses #816

Closed
wants to merge 1 commit into from

Conversation

magnetised
Copy link
Contributor

Fixes #815

Copy link
Member

@mreiferson mreiferson left a comment

Choose a reason for hiding this comment

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

@magnetised thanks for this, just one minor comment

@@ -77,11 +78,29 @@ func (p *Producer) Address() string {
}

func (p *Producer) HTTPAddress() string {
return fmt.Sprintf("%s:%d", p.BroadcastAddress, p.HTTPPort)
return addressWithPort(p.BroadcastAddress, p.HTTPPort)
Copy link
Member

Choose a reason for hiding this comment

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

go has builtins for this, let's use net.JoinHostPort

@magnetised
Copy link
Contributor Author

@mreiferson Done. standard library FTW.

@mreiferson
Copy link
Member

Re-triggered those flakey test runs. In the meantime would you mind squashing down to one commit?

@mreiferson
Copy link
Member

apparently I don't know how to use git or github anymore ^^^

@mreiferson
Copy link
Member

@magnetised thanks!

@magnetised
Copy link
Contributor Author

@mreiferson I'd forgotten all about this - thanks for the unintentional email-storm reminder :)

@magnetised
Copy link
Contributor Author

@mreiferson any idea when the next release is?

@mreiferson
Copy link
Member

@mreiferson any idea when the next release is?

Trying to get things in shape for one!

@magnetised
Copy link
Contributor Author

@mreiferson great. good luck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants