-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
feat!: Provide logger through depinject #15818
Conversation
i dont think this is needed, we can just pass the logger from cosmossdk.io/logger which is instantiated already |
This comment has been minimized.
This comment has been minimized.
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.
Mostly lgtm. Is there an issue to track the fact that we should do that for all modules?
Co-authored-by: Julien Robert <[email protected]>
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! But I think still we need an issue for not forgetting to do that in other modules.
Co-authored-by: Julien Robert <[email protected]>
Description
What is proposed here is that we pass in the logger using
depinject.Supply
instead of through theAppBuilder
'sBuild
method. The bank module was modified as an example.Ref: #14683
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change