feat: add TransactionContext

[kevinschoonover]

This PR

• Adds backwards compatible, basic implementation of 'TransactionContext'

The functions that are important are

WithTranscationContext(ctx, EvaluationContext)
TransactionContext(ctx)

Related Issues

#256

Notes

For next steps,

• I see the API suggested in [FEATURE] Implement transaction context #256 is to remove the local evalCtx which would make the API much cleaner in my opinion, but I am not sure what is the process for breaking changes
• I am also not sure what the right behavior of merging should be from the API's perspective if they run WithTransactionContext multiple times. My guess is we should expect the user to handle this on their end, but would be interested in opinions on if WithTransactionContext should merge existing TransactionContexts

How to test

I updated the associated tests for merging order / evaluation. Please let me know if any other tests will be helpful!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.12%. Comparing base (ad9db29) to head (fe4b61d).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #283      +/-   ##
==========================================
+ Coverage   81.92%   82.12%   +0.20%     
==========================================
  Files          10       10              
  Lines         979      990      +11     
==========================================
+ Hits          802      813      +11     
  Misses        156      156              
  Partials       21       21              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[beeme1mr]

Hey @kevinschoonover, could you please update the readme to include a section about this? Please follow the template found here. Thanks!

[beeme1mr]

I am also not sure what the right behavior of merging should be from the API's perspective if they run WithTransactionContext multiple times. My guess is we should expect the user to handle this on their end, but would be interested in opinions on if WithTransactionContext should merge existing TransactionContexts

Yes, I agree. Currently, all implementations override existing transaction context, but adding another method that merges would be useful.

[kevinschoonover]

Hey @kevinschoonover, could you please update the readme to include a section about this? Please follow the template found here. Thanks!

Done!

I am also not sure what the right behavior of merging should be from the API's perspective if they run WithTransactionContext multiple times. My guess is we should expect the user to handle this on their end, but would be interested in opinions on if WithTransactionContext should merge existing TransactionContexts

Yes, I agree. Currently, all implementations override existing transaction context, but adding another method that merges would be useful.

Added a MergeTransactionContext function which should solve this case. Let me know if there is anything else I can update!

[beeme1mr]

This looks great, thanks! I'll defer the final review to a Go expert but it looks good to me.

[evan-bradley]

This looks good to me. All my suggestions are non-blocking.

[lukas-reining]

Looks really good to me!