-
Notifications
You must be signed in to change notification settings - Fork 949
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
bugfix: move function getListener() to pkg/net/listener.go #2040
Conversation
pkg/net/listener.go
Outdated
func GetListener(addr string, tlsConfig *tls.Config) (net.Listener, error) { | ||
addrParts := strings.SplitN(addr, "://", 2) | ||
if len(addrParts) != 2 { | ||
return nil, fmt.Errorf("invalid listening address: %s", addr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in a common function, we could give more details about the error message. How about adding like this:
fmt.Errorf("invalid listening address %s: address must be [protocol]://[address]", addr)
06214a2
to
efed1e5
Compare
Codecov Report
@@ Coverage Diff @@
## master #2040 +/- ##
===========================================
+ Coverage 51.18% 63.78% +12.59%
===========================================
Files 200 201 +1
Lines 15531 15527 -4
===========================================
+ Hits 7950 9904 +1954
+ Misses 6513 4385 -2128
- Partials 1068 1238 +170
|
7e8a6be
to
ead57c2
Compare
@fuweid PTAL. |
pkg/netutils/listener_test.go
Outdated
|
||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
_, err := GetListener(tt.args.addr, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we close it if the case pass?
Signed-off-by: YaoZengzeng <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: YaoZengzeng [email protected]
Ⅰ. Describe what this PR did
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews