-
Notifications
You must be signed in to change notification settings - Fork 493
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
Adding "if" function to tick #745
Conversation
|
||
func (*ifFunc) Call(args ...interface{}) (interface{}, error) { | ||
if len(args) != 3 { | ||
return nil, errors.New("if expects exactly three argument") |
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.
nit: Can you change argument
to arguments
?
@yosiat Looks great! A few small questions but overall this looks ready to go. |
49f14d4
to
f1f90d6
Compare
@nathanielc fixed everything 👍 |
} | ||
|
||
if reflect.TypeOf(args[1]) != reflect.TypeOf(args[2]) { | ||
return nil, fmt.Errorf("Multiple return types isn't supported - second argument is %T and third argument is %T", args[1], args[2]) |
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.
nit:
I think this would read better if it read: Different return types are not supported - ...
// multiple types | ||
{ | ||
args: []interface{}{false, 1, "1"}, | ||
err: errors.New("Multiple return types isn't supported - second argument is int and third argument is 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.
Tests are failing on this error message.
Required for all non-trivial PRs
Fixes #741