Refactor getOne out of GetSCTs #8469
Conversation
This is a small refactor to remove a pair of closures so we can directly launch the submission goroutines with `go ctp.getOne(...)`. This is itself pulled out of a slightly larger refactor which pulls the stagger sleep back out of this function, but it's much cleaner to do this refactor as a preliminary change. There is a small functional change here: The same subCtx is passed to the submission RPC. I think that shouldn't cause any problems.
|
Marking this as a draft for now so I can get the follow-up PR ready, which will make this one make more sense. |
aarongable
left a comment
aarongable
left a comment
There was a problem hiding this comment.
I am curious to see what the follow-on refactor is. I'm totally on board with pulling getOne out to be a helper rather than a closure, but I'm not a big fan of changing its idiomatic (result, error) return into a non-idomatic and future-error-prone "you have to remember to empty-return after writing to the results channel".
|
Opened as #8470 I think the main thing is that having multiple call sites to getOne means that there's no duplication of the logic at the call site. I don't 100% love it, so am definitely open to suggestions. |
|
I'm actually no longer convinced this has much extra reviewability over #8470, so I'll just close this and target that to main |
This is a small refactor to remove a pair of closures so we can directly launch the submission goroutines with
go ctp.getOne(...).This is itself pulled out of a slightly larger refactor which pulls the stagger sleep back out of this function, but it's much cleaner to do this refactor as a preliminary change.
There is a small functional change here: The same subCtx is passed to the submission RPC. I think that shouldn't cause any problems.