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

[v1, 2/8] Export the Encoder type #113

Merged
merged 1 commit into from
Aug 1, 2016
Merged

Conversation

akshayjshah
Copy link
Contributor

Export the Encoder type, since we'd like to allow external implementations. (Exporting a constructor for the JSON encoder comes in the next PR.)

This is the second of eight PRs to finish up the large pre-v1.0.0 refactoring:

  • Meta constructor takes an encoder
  • Export Encoder type
  • Export the JSON encoder constructor
  • Add a NewLogger constructor and remove NewJSON
  • Remove StubTime
  • Export the Hook type
  • Remove Encoder.AddFields (since we already have Field.AddTo)
  • Improve GoDoc.

The remaining items in the v1 milestone shouldn't require large-scale changes.

Since zap's logger is now strongly separated from its encoders, we need
to export the encoder type so users can construct loggers.
// safe for concurrent use.
type encoder interface {
type Encoder interface {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we are exporting this, let's add comments on the below methods, especially Clone/Free since we're expecting the implementors to do the pooling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's covered in PR 8/8, if that's okay.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@prashantv
Copy link
Collaborator

lgtm

@akshayjshah akshayjshah merged commit 79a7630 into master Aug 1, 2016
@akshayjshah akshayjshah deleted the ajs-export-encoder branch August 1, 2016 16:10
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.

None yet

2 participants