Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Initializing logger for each command #3090

Merged
merged 3 commits into from
Nov 2, 2016
Merged

Initializing logger for each command #3090

merged 3 commits into from
Nov 2, 2016

Conversation

tomusdrw
Copy link
Collaborator

@tomusdrw tomusdrw commented Nov 2, 2016

Fixes #3087

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Nov 2, 2016
Cmd::Blockchain(blockchain_cmd) => blockchain::execute(blockchain_cmd),
Cmd::SignerToken(path) => signer::new_token(path),
Cmd::Snapshot(snapshot_cmd) => snapshot::execute(snapshot_cmd),
Cmd::Version(log) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

how about:

match command {
    Cmd::Run(..) => {},
    _ => setup_log(&log).expect("Logger is initialized only once; qed"),
};

immediately prior to the original code?

Copy link
Contributor

Choose a reason for hiding this comment

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

or even just pulling the logger setup out of run::execute and then there's no need for this new match, either.

@gavofyork gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 2, 2016
@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. A8-looksgood 🦄 Pull request is reviewed well. labels Nov 2, 2016
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Nov 2, 2016
@gavofyork
Copy link
Contributor

failing tests seem to be unrelated.

@gavofyork gavofyork merged commit cf8f27c into master Nov 2, 2016
@gavofyork gavofyork deleted the logger-fix branch November 2, 2016 18:42
@rphmeier
Copy link
Contributor

rphmeier commented Nov 2, 2016

on appveyor it seems related: with_color: true vs with_color: false. This is relevant to this PR, right?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants