Skip to content

Conversation

@gabriel-indik
Copy link
Contributor

Signed-off-by: Gabriel Indik gabriel.indik@kaleido.io

Signed-off-by: Gabriel Indik <gabriel.indik@kaleido.io>
Signed-off-by: Gabriel Indik <gabriel.indik@kaleido.io>
@codecov-commenter
Copy link

Codecov Report

Merging #534 (ac265c9) into main (fe435c1) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #534   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          276       276           
  Lines        15918     15952   +34     
=========================================
+ Hits         15918     15952   +34     
Impacted Files Coverage Δ
internal/dataexchange/ffdx/ffdx.go 100.00% <100.00%> (ø)
internal/events/dx_callbacks.go 100.00% <100.00%> (ø)
internal/privatemessaging/privatemessaging.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe435c1...ac265c9. Read the comment docs.

Copy link
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks @gabriel-indik for completing the lifecycle here.

fyi @awrichar that @gabriel-indik and I discussed a follow-on piece of work, to allow a DX plugin (like the ffdx connector interface) to be able to use the output from a previous operation update, to combine with a subsequent update.

e.g. the message-delivered output contains some information about the request, and the DX connector is stateless. So when the message-acknowledged update comes back, it doesn't want to overwrite the operation - rather it wants to combine together the JSONObject fields in some way determined by the Go plugin.

Feels like a positive change and valid requirement to me, but I suggested it's best to let the dust settle on your refactoring work first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants