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 example that shows how to log to a file #101

Merged
merged 1 commit into from
Jul 25, 2016
Merged

Conversation

prashantv
Copy link
Collaborator

Fixes #100

@@ -54,6 +55,35 @@ func Example() {
// {"msg":"Oh no!","level":"error","ts":0,"fields":{"user":"jane@test.com","visits":42}}
}

func Example_fileOutput() {
// Create a temporary file to output logs to.
f, err := ioutil.TempFile("", "log")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't you need to delete the file after you're done?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, added a defer os.Remove(...)

@abhinav
Copy link
Collaborator

abhinav commented Jul 25, 2016

LGTM


logger.Info("This is an info log.", zap.Int("foo", 42))

// Sync the file so logs are written to disk, and print the file contents.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're including this in an example, can we call out that zap already syncs when logging at PanicLevel and FatalLevel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, added

@akshayjshah
Copy link
Contributor

:shipit:

@prashantv prashantv merged commit 02d453c into master Jul 25, 2016
@prashantv prashantv deleted the pv_fileexample branch July 25, 2016 22:07
}
defer os.Remove(f.Name())

logger := zap.NewJSON(

Choose a reason for hiding this comment

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

zap do not have NewJSON method?
what should I do?

@akshayjshah
Copy link
Contributor

@MonsieurHorse let's discuss this on the issue you opened. This PR is very old, and APIs have changed quite a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants