Skip to content
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

Replace fmt.Printf with an actual logger #14

Closed
ErinCall opened this issue Dec 17, 2019 · 1 comment
Closed

Replace fmt.Printf with an actual logger #14

ErinCall opened this issue Dec 17, 2019 · 1 comment
Labels
code quality Improvements to the internals that may not have user-facing impact good first issue Good for newcomers

Comments

@ErinCall
Copy link
Contributor

Currently, all logging is done with fmt.Printf (or .Println, .Fprintf, etc.). That's fine for a first pass, but we should really use the log package or something like it. Printf output can pollute the test output, and it would be great to be able to say logger.Debug("...") instead of

if cfg.Debug {
	fmt.Fprint(cfg.Stderr, "...\n")
}
@ErinCall ErinCall added good first issue Good for newcomers code quality Improvements to the internals that may not have user-facing impact labels Dec 17, 2019
@ErinCall
Copy link
Contributor Author

ErinCall commented Jan 1, 2020

Closing this as wontfix—I had succumbed to the temptation to overdesign.

I made some progress using Logrus, but hooking up commands' stdout/stderr to the logger turns out to be surprisingly complex. I don't think the effort is worth the trouble.

@ErinCall ErinCall closed this as completed Jan 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Improvements to the internals that may not have user-facing impact good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant