-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Added configmap option to turn on proxy protocol and set_real_ip_from #81
Conversation
@thetechnick thx for another pull request! Couple of suggestions:
expected := []string{"1.String","2.String","3.String"}
fmt.Println(expected)
if !reflect.DeepEqual(expected, slice) {
t.Errorf("Unexpected return value:\nGot: %v\nExpected: %v", slice, expected)
}
|
@pleshakov Edit: |
c8e62cf
to
1755d28
Compare
@thetechnick thx. Looks good! I added few comments about the unit test. |
@pleshakov |
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 added few comments about the unit test.
if !exists { | ||
t.Errorf("The key 'key' must exist in the configMap") | ||
} | ||
expected := []string{"1.String,2.String,3.String"} |
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.
The expected value should be expected := []string{"1.String", "2.String", "3.String"}
In this case, the condition in line 176 should be changed to if !reflect.DeepEqual(expected, slice)
expected := []string{"1.String,2.String,3.String"} | ||
t.Log(expected) | ||
if reflect.DeepEqual(expected, slice) { | ||
t.Errorf("Unexpected return value:\nGot: %v\nExpected: %v", slice, expected) |
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.
It's better to print the slices as %#v a Go-syntax representation of the value
t.Errorf("Unexpected return value:\nGot: %#v\nExpected: %#v", slice, expected)
In your case, both []string{"1.String,2.String,3.String"}
and []string{"1.String", "2.String", "3.String"}
are printed as [1.String 2.String 3.String]
, which can lead to confusion:
convert_test.go:177: Unexpected return value:
Got: [1.String 2.String 3.String]
Expected: [1.String 2.String 3.String]
My apologies. I forget to publish the comments |
@pleshakov |
@thetechnick Great! Could you squash the two commits? |
7e1b114
to
c38bad1
Compare
@pleshakov done |
@thetechnick thx! |
As discussed in #73