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

Justify slog's existence #53

Merged
merged 4 commits into from
Dec 4, 2019
Merged

Justify slog's existence #53

merged 4 commits into from
Dec 4, 2019

Conversation

nhooyr
Copy link
Contributor

@nhooyr nhooyr commented Nov 27, 2019

Closes #6
Closes #34

@nhooyr nhooyr changed the title <!-- Please read the contributing guidelines. https://github.com/cdr/slog/blob/master/.github/CONTRIBUTING.md#pull-requests --> Improve docs Nov 27, 2019
@nhooyr nhooyr changed the title Improve docs Justify slog's existence Nov 27, 2019
Copy link
Member

@coadler coadler left a comment

Choose a reason for hiding this comment

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

Very nice writeup

README.md Outdated
We wanted an API that only accepted the equivalent of [zap.Any](https://godoc.org/go.uber.org/zap#Any) for every field.
This is [slog.F](https://godoc.org/cdr.dev/slog#F).

Second, we found the human readable format to be hard to read due to the lack appropriate colors for different levels
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Second, we found the human readable format to be hard to read due to the lack appropriate colors for different levels
Second, we found the human readable format to be hard to read due to the lack of appropriate colors for different levels

README.md Outdated
instead. We wanted it to be automatic, even for private fields. slog handles this transparently for us. It will automatically
log Go structures with their fields separate, including private fields.

Seventh, t.Helper style APIf
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Seventh, t.Helper style APIf
Seventh, t.Helper style API

README.md Outdated

Seventh, t.Helper style APIf

Eight, tighter integration with `*testing.T`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Eight, tighter integration with `*testing.T`.
Eighth, tighter integration with `*testing.T`.

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

Successfully merging this pull request may close these issues.

Document how the reflection encoding works Compare to zap in README.md
2 participants