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 APIs to support buffering #364

Merged
merged 2 commits into from
Mar 10, 2017
Merged

Add APIs to support buffering #364

merged 2 commits into from
Mar 10, 2017

Conversation

akshayjshah
Copy link
Contributor

This is a breaking change that adds a Sync method to zapcore.Core. The
intention here is to expose Sync all the way up to the loggers themselves;
this lets implementations buffer I/O, since the end user can Sync at the end
of main.

(Of course, nothing can save users if they directly panic or os.Exit from a
different goroutine.)

This fixes #355.

@mention-bot
Copy link

@akshayjshah, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jcorbin, @prashantv and @akabos to be potential reviewers.

This is a breaking change that adds a `Sync` method to `zapcore.Core`. The
intention here is to expose `Sync` all the way up to the loggers themselves;
this lets implementations buffer I/O, since the end user can `Sync` at the end
of main.

(Of course, nothing can save users if they directly panic or os.Exit from a
different goroutine.)
@akshayjshah akshayjshah merged commit ed01b7b into master Mar 10, 2017
@prashantv prashantv deleted the ajs-buffer branch March 10, 2017 16:44
@jcorbin
Copy link
Contributor

jcorbin commented Mar 10, 2017

Given that we frequently are writing docs like "Sync flushes..." perhaps the better name here would've indeed been Flush() error ?

@akshayjshah
Copy link
Contributor Author

Flush is definitely better on the loggers, but Sync is better on the WriteSyncer. Keeping the names the same clearly indicates that they're related, which I weighed more heavily than the name itself.

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.

Support offloading I/O to a separate goroutine
4 participants