refactor: add logger package #14872
refactor: add logger package #14872
Conversation
github-prbot
requested review from
a team,
JeancarloBarrios and
atheeshp
and removed request for
a team
| @@ -0,0 +1,3 @@ | |||
| module github.com/cosmos/cosmos-sdk/log | |||
There was a problem hiding this comment.
Given that it is a small PR, could you add all the tooling too?
There was a problem hiding this comment.
Just one typo, and could you add it to dependabot?
There was a problem hiding this comment.
Should we make it cosmossdk.io/log? Why not just have this interface in core? I think there should be a core logging service
There was a problem hiding this comment.
I thought core should not be containing any implementation.
If you had an interface in core and the implementation in logger that's a bit weird. Imo better to keep close the interface from its implementation for small packages like that.
Agrees that a vanity url is better however.
There was a problem hiding this comment.
I think a no-op implementation in core is fine. That's all this has right? I don't have a strong preference on whether it goes in core, but I do think modules will want a logging service so that should probably go in core right?
There was a problem hiding this comment.
Right, but maybe in that log package we can add a function for these logger too, so we don't duplicate other loggers:
- https://github.com/cosmos/cosmos-sdk/blob/main/tools/cosmovisor/logger.go
- https://github.com/cosmos/cosmos-sdk/blob/main/tools/rosetta/lib/logger/logger.go
This was what makes the most sense to me, imo as this does not fit in core.
There was a problem hiding this comment.
No, that probably shouldn't be in core. So if we can have a simple interface with zero dependencies, the same interface can exist in core and log. Log can provide some implementations like zerolog and comet. And core can have the same interface for the service but no implementations. The main thing I think is getting rid of the dependency on comet as I mentioned here #14904 (comment)
tac0turtle
force-pushed
the
marko/logger_pacakge
branch
from
6c0c828 to
386a02e
Compare
| @@ -0,0 +1,14 @@ | |||
| sonar.projectKey=cosmos-sdk-log | |||
There was a problem hiding this comment.
Created 👍🏾 -> https://sonarcloud.io/project/overview?id=cosmos-sdk-log
|
[Cosmos SDK - Log] Kudos, SonarCloud Quality Gate passed!
|
Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: Julien Robert <julien@rbrt.fr>
Description
moves logger to its own package and adds a NoOp logger
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.mdReviewers 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