Skip to content

pass logger config to mockserver#553

Merged
tzdybal merged 1 commit intoevstack:mainfrom
randomshinichi:passloggerconfig
Oct 13, 2022
Merged

pass logger config to mockserver#553
tzdybal merged 1 commit intoevstack:mainfrom
randomshinichi:passloggerconfig

Conversation

@randomshinichi
Copy link
Copy Markdown
Contributor

closes #363

Copy link
Copy Markdown
Contributor

@tzdybal tzdybal left a comment

Choose a reason for hiding this comment

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

Thanks for your submission!

Usually logger is passed as last argument in our codebase, but it doesn't really matter.

@randomshinichi
Copy link
Copy Markdown
Contributor Author

randomshinichi commented Oct 11, 2022

Usually logger is passed as last argument in our codebase, but it doesn't really matter.
I can do that... just pushed

Copy link
Copy Markdown
Contributor

@tzdybal tzdybal left a comment

Choose a reason for hiding this comment

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

Thanks for taking a time to fix the nit-pick comment.

There is one tiny issue with imports, I'll update linter to automatically catch such cases.

}
log.Println("Listening on:", lis.Addr())
srv := mockserv.GetServer(kv, conf, nil)
srv := mockserv.GetServer(kv, conf, nil, logger)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

grpcda "github.com/celestiaorg/rollmint/da/grpc"
"github.com/celestiaorg/rollmint/da/grpc/mockserv"
"github.com/celestiaorg/rollmint/store"
tmlog "github.com/tendermint/tendermint/libs/log"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is mixing "local" and 3rd party imports. tmlog should be imported in separate "section" before all the rollmint imports.

This should be detected by linter.

@tzdybal tzdybal merged commit c1b6483 into evstack:main Oct 13, 2022
@tzdybal tzdybal mentioned this pull request Oct 13, 2022
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.

Pass logger config to mockserver

2 participants