-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Refactor Encoder.WriteEntry => Encoder.EncodeEntry #245
Conversation
@jcorbin, thanks for your PR! By analyzing the history of the files in this pull request, we identified @akshayjshah and @prashantv to be potential reviewers. |
@@ -113,7 +113,7 @@ func TestWriterFacilitySyncsOutput(t *testing.T) { | |||
} | |||
} | |||
|
|||
func TestWriterFacilityWriteEntryFailure(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add another test case here with the short writer from the testutils package?
} | ||
|
||
final := jsonPool.Get().(*jsonEncoder) | ||
final.truncate() | ||
final.bytes = append(final.bytes, '{') | ||
enc.LevelFormatter(ent.Level).AddTo(final) | ||
enc.TimeFormatter(ent.Time).AddTo(final) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for sanity, we should probably use final.LevelFormatter
. Same for the time and message formatters.
func TestWriterFacilityShortWrite(t *testing.T) { | ||
fac := WriterFacility( | ||
NewJSONEncoder(testJSONConfig()), | ||
Lock(AddSync(&testutils.ShortWriter{})), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate note: we don't need the the AddSync
anymore.
7fe5b84
to
92dbcaa
Compare
* Refactor Encoder.{Write => Encode}Entry * Just use final in jsonEncoder.EncodeEntry * Add a short write test * Fix dat package * Drop AddSync from TestWriterFacility*
Encoders should, well, encode. Writing to a sink is a concern for the facility.