Split definition methods out of Broadcast Manager into new Definition Sender #790
Conversation
… Sender Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
| @@ -53,57 +88,36 @@ func (bm *broadcastManager) BroadcastIdentityClaim(ctx context.Context, ns strin | |||
| return bm.broadcastDefinitionCommon(ctx, ns, def, signingIdentity, tag, waitConfirm) | |||
| } | |||
|
|
|||
| func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns string, def fftypes.Definition, signingIdentity *fftypes.SignerRef, tag string, waitConfirm bool) (*fftypes.Message, error) { | |||
| func (bm *definitionSender) broadcastDefinitionCommon(ctx context.Context, ns string, def fftypes.Definition, signingIdentity *fftypes.SignerRef, tag string, waitConfirm bool) (*fftypes.Message, error) { | |||
There was a problem hiding this comment.
Just want to note that this is probably the only method with functional changes beyond simple renaming. Due to the new separation between broadcast manager and definition sender, less of the data processing is done directly. We simply build a MessageInOut with InlineData, similar to any other broadcast. It feels clean overall, but I just wanted to point it out among the noise.
Codecov Report
@@ Coverage Diff @@
## main #790 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 323 323
Lines 20195 20206 +11
=========================================
+ Hits 20195 20206 +11
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Just checking @awrichar - could DefinitionSender go into the broadcast package? (hence leaving the definitions package with its current name, but only containing the handlers)
|
Yea, it probably could. But now I'm thinking it's best to pause on this until FIR-12 moves along a little further. The thought is that soon definitions will not necessarily be a broadcast, so they don't totally belong in the broadcast package. Going to wait until that next piece of work queues up though before doing any more shuffling here. |
|
Converted to draft based on last comment |
|
Replaced by #878 |
Not totally crazy about the names
defsender.Senderanddefhandler.DefinitionHandler, but they need to be separate packages and this is where I landed...