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

add package for logging output #248

Closed
wants to merge 1 commit into from
Closed

Conversation

lcowell
Copy link
Contributor

@lcowell lcowell commented Nov 9, 2015

No description provided.

@lcowell
Copy link
Contributor Author

lcowell commented Nov 9, 2015

This will provide us with 2 levels of output, normal output and debugging output. With the Verbose flag enabled, debugging output will also be shown to the user. Once we're happy with how this is looking, I'll integrate it with the rest of the app.

@kytrinyx
Copy link
Member

I think I like where this is going. I'm hesitating over one thing: do we really want to use the log package to tell the user things? That seems like not logging to me. Now that I write it out, it seems like debug info isn't quite logging either, so maybe this is just a question of semantics.

@lcowell
Copy link
Contributor Author

lcowell commented Nov 11, 2015

Semantics are important. The better the naming, the less we have to think when we're reading code. What do you think about changing the package name to out or output?

out.Println("hi")
out.Debugf("%s\n", "ok")

We could also keep the user related output going through the fmt package and make a debug package. So we'd have debug.Println and that would only print if verbose (or perhaps should be rename to debug) flag is set.

I'm kind of leaning toward the debug package as it doesn't interfere with normal user output.

@kytrinyx
Copy link
Member

Yeah, either of these feel like good approaches to me. I'm with you on leaning a bit towards using fmt.Print directly for normal user output, and then having a debug package for the verbose output.

I'm thinking the usual stuff would go to STDOUT and the debug stuff would go to STDERR.

@lcowell lcowell force-pushed the logging branch 3 times, most recently from f68b71a to 833cba9 Compare November 16, 2015 00:02
This package will print output to StdErr when the verbose flag is set for the cli.
@lcowell
Copy link
Contributor Author

lcowell commented Nov 16, 2015

This should now reflect the changes we'd talked about. I've also wired up the verbose flag to activate the debug output.

EDIT: something went screwy when I tried to rebase. I'll cherry-pick this commit onto master if this looks good.

@Tonkpils
Copy link
Contributor

👍 Looks good to me.

@lcowell lcowell closed this Nov 16, 2015
@lcowell
Copy link
Contributor Author

lcowell commented Nov 16, 2015

merged e071831

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.

3 participants