refactor inspector helper to use iterators instead of callbacks - #248
refactor inspector helper to use iterators instead of callbacks#248sivchari wants to merge 1 commit into
Conversation
Signed-off-by: sivchari <shibuuuu5@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sivchari The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
JoelSpeed
left a comment
JoelSpeed
left a comment
There was a problem hiding this comment.
LGTM, just one question about whether we actually still need that panic recovery at all now
| defer func() { | ||
| if r := recover(); r != nil { | ||
| // If the inspectField function panics, we recover and log information that will help identify the issue. | ||
| // If the consumer panics, we recover and log information that will help identify the issue. | ||
| debug := printDebugInfo(field) | ||
| panic(fmt.Sprintf("%s %v", debug, r)) // Re-panic to propagate the error. | ||
| } | ||
| }() |
There was a problem hiding this comment.
I'm not convinced we will need this recovery now, since the panic is not handled in a closure we call, will this actually do anything?
There was a problem hiding this comment.
Verified this still works correctly, since yield() is called synchronously within this function's stack frame, the deferred recover() catches panics from the loop body just like before.
|
Hi @JoelSpeed |
3 participants
Introduce iterator pattern instead of callbacks. It's faster and safer than current implementation.