-
Notifications
You must be signed in to change notification settings - Fork 215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(abci): Pass the context parameter #2576
Conversation
@@ -174,7 +174,7 @@ func (s *Service) Start( | |||
return err | |||
} | |||
|
|||
s.ctx = ctx | |||
s.ResetAppCtx(ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also just a nit. Might as well use the setter function.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2576 +/- ##
==========================================
- Coverage 58.14% 57.94% -0.21%
==========================================
Files 335 338 +3
Lines 15036 15122 +86
Branches 0 20 +20
==========================================
+ Hits 8743 8762 +19
- Misses 5632 5698 +66
- Partials 661 662 +1
🚀 New features to boost your workflow:
|
@@ -46,7 +46,7 @@ func (s *Service) InitChain( | |||
return &cmtabci.InitChainResponse{}, s.ctx.Err() | |||
} | |||
//nolint:contextcheck // see s.ctx comment for more details | |||
return s.initChain(req) // internally this uses s.ctx | |||
return s.initChain(s.ctx, req) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this PR, I could easily interchange this s.ctx
with the ctx
that is provided in the function parameters.
Also note that on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Passing the beacond App's
s.ctx
context field as a parameter to the ABCI functions imitates the functionality of using thectx
from CometBFT. This makes it so that the arguments in each ABCI function call are easily swappable betweens.ctx
andctx
, should CometBFT ever fully implement theirctx
.