-
Notifications
You must be signed in to change notification settings - Fork 125
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
fix bug and delete String of Int and Float #205
Conversation
validator/vars.go
Outdated
return val, nil | ||
} | ||
case "Float": | ||
if kind == reflect.String || kind == reflect.Float32 || kind == reflect.Float64 || kind == reflect.Int || kind == reflect.Int32 || kind == reflect.Int64 { | ||
if kind == reflect.Float32 || kind == reflect.Float64 || kind == reflect.Int || kind == reflect.Int32 || kind == reflect.Int64 { |
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 am not sure why you removed the kind == reflect.String
condition as "45.230343" strings are often how they are serialized.
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 tests return an error because you are giving a string to an Int.
I removed the string from the int and float because if you set a variable as int or float you should get an int or a float, not a string, in fact, I got errors for that too. I hope it is useful for something.
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.
still, adding the string without removing the validation on the Int, there is no problem. the bad thing is that it can return a string in an int or float.
With this change, the tests fail with:
|
ast/document_test.go
Outdated
@@ -6,8 +6,8 @@ import ( | |||
"github.com/stretchr/testify/assert" | |||
"github.com/stretchr/testify/require" | |||
|
|||
. "github.com/vektah/gqlparser/v2/ast" |
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.
Hi, can you fix this back?
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.
sure!
Signed-off-by: Steve Coffman <[email protected]>
So there was a small performance regression with your changes that I corrected by short circuiting for the most common cases. Thanks! |
repair error "cannot use float64 as Int" issue #204 and delete string into Int and Float