-
Notifications
You must be signed in to change notification settings - Fork 20
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
Update example to use new func RegisterWithErrorFunc #42
Conversation
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.
Thanks for your contribution.
Could you bring back the error handling and sign your commit, please?
@@ -42,11 +43,10 @@ func ExampleNfqueue_Register() { | |||
} | |||
|
|||
// Register your function to listen on nflqueue queue 100 | |||
err = nf.Register(ctx, fn) |
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.
Replacing the deprecated function Register()
with RegisterWithErrorFunc()
in the example is a good idea. But we should keep the error handling.
b5c05a1
to
fee052f
Compare
done! |
example_test.go
Outdated
err = nf.RegisterWithErrorFunc(ctx, fn, func(e error) int { | ||
if err != nil { | ||
fmt.Println(err) | ||
return -1 | ||
} | ||
return 0 | ||
}) |
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.
If ErrorFunc is called in the scope of RegisterWithErrorFunc()
, then error is never nil.
But RegisterWithErrorFunc()
also returns an error, e.g. if the hook can not be registered to the socket. Then this error should not be ignored.
err = nf.RegisterWithErrorFunc(ctx, fn, func(e error) int { | |
if err != nil { | |
fmt.Println(err) | |
return -1 | |
} | |
return 0 | |
}) | |
err = nf.RegisterWithErrorFunc(ctx, fn, func(e error) int { | |
fmt.Println(err) | |
return -1 | |
}) | |
if err != nil { | |
fmt.Println(err) | |
return | |
} |
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.
Sorry, I misunderstood your original request. Changed as suggested.
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.
Thanks for the update - it was my mistake not to be precious about my requested changes.
Signed-off-by: giuliano <[email protected]>
No description provided.