Skip to content

Commit

Permalink
Improve comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Akshay Shah committed Sep 18, 2017
1 parent 2fa2ec2 commit 002b6ce
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 4 deletions.
5 changes: 4 additions & 1 deletion zbark/barkify.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ func (l barker) WithError(err error) bark.Logger {
}

func (l barker) Fields() bark.Fields {
l.SugaredLogger.Warn("Fields() call to bark logger is not supported by Zap")
// Zap has already marshaled the accumulated logger context to []byte, so we
// can't reconstruct the original objects. To satisfy this interface, just
// return nil.
l.SugaredLogger.Warn("zap-to-bark compatibility wrapper does not support Fields method")
return nil
}
2 changes: 1 addition & 1 deletion zbark/barkify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestBarkLoggerWith(t *testing.T) {

entry := logs.All()[0]
assert.Equal(t,
"Fields() call to bark logger is not supported by Zap", entry.Message,
"zap-to-bark compatibility wrapper does not support Fields method", entry.Message,
"message did not match")
assert.Equal(t, zapcore.WarnLevel, entry.Level, "message level did not match")
})
Expand Down
11 changes: 9 additions & 2 deletions zbark/zapify.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (

// Zapify wraps a bark logger in a compatibility layer to produce a
// *zap.Logger. Note that the wrapper always treats zap's DPanicLevel as an
// error (even in production).
// error (even in development).
func Zapify(l bark.Logger) *zap.Logger {
return zap.New(&zapper{l})
}
Expand All @@ -38,6 +38,10 @@ type zapper struct {
}

func (z *zapper) Enabled(lvl zapcore.Level) bool {
// Enabled allows zap to short-circuit some logging calls early, which
// improves performance. Since we're not sure what levels are enabled on the
// underlying bark logger, always return true; this hurts performance but
// not correctness.
return true
}

Expand All @@ -46,6 +50,7 @@ func (z *zapper) With(fs []zapcore.Field) zapcore.Core {
}

func (z *zapper) Check(ent zapcore.Entry, ce *zapcore.CheckedEntry) *zapcore.CheckedEntry {
// Since Enabled is always true, there's no need to check it here.
return ce.AddCore(ent, z)
}

Expand All @@ -69,12 +74,14 @@ func (z *zapper) Write(ent zapcore.Entry, fs []zapcore.Field) error {
default:
return fmt.Errorf("bark-to-zap compatibility wrapper got unknown level %v", ent.Level)
}
// The underlying bark logger timestamps the entry.
// The underlying bark logger timestamps the entry, so we can drop
// everything but the message.
logFunc(ent.Message)
return nil
}

func (z *zapper) Sync() error {
// Bark doesn't expose a way to flush buffered messages.
return nil
}

Expand Down

0 comments on commit 002b6ce

Please sign in to comment.