docs: interoperability with slog #222
Conversation
58315c2 to
e164f61
Compare
| @@ -21,7 +21,8 @@ limitations under the License. | |||
| // API and of a logr.LogSink through the slog.Handler and thus slog.Logger | |||
| // APIs. | |||
| // | |||
| // Both approaches are currently experimental and need further work. | |||
| // See the README in the top-level [./logr] package for a discussion of | |||
| // interoperability. | |||
There was a problem hiding this comment.
My hope is that pkg.go.dev will render this as a link. I've not tried to verify that beforehand because it's not that simple anymore, with godoc being deprecated...
There was a problem hiding this comment.
I can't get pksite to work - it seems to look at github for something, and refuses to include the ./slogr directory
There was a problem hiding this comment.
I haven't tried at all. I guess we'll have to hope for the best and check after a release.
10757e8 to
1d7c75d
Compare
|
@mengjiao-liu: Have you had a chance to catch up on the log/slog interoperability discussion? This PR and the commits in master since the last release are good opportunity to catch up. Your review comments on this PR would also be useful. |
|
I didn't mean to quietly kill #213 - I have just had zero free moments to think about it. Is this PR a case of "oh well, let's just document it" ? |
README.md
Outdated
| Not supporting slog has several drawbacks: | ||
| - Recording source code locations works correctly if the handler gets called | ||
| through `slog.Logger`, but may be wrong in other cases. That's because a | ||
| `slog.Sink` does its own stack unwinding instead of using the program counter |
There was a problem hiding this comment.
slog.Sink is not a type I know? Do yhou mean logr.Sink ?
There was a problem hiding this comment.
Right, that should have been logr.Sink - fixed.
| @@ -21,7 +21,8 @@ limitations under the License. | |||
| // API and of a logr.LogSink through the slog.Handler and thus slog.Logger | |||
| // APIs. | |||
| // | |||
| // Both approaches are currently experimental and need further work. | |||
| // See the README in the top-level [./logr] package for a discussion of | |||
| // interoperability. | |||
There was a problem hiding this comment.
I can't get pksite to work - it seems to look at github for something, and refuses to include the ./slogr directory
It documents the current status, so we can merge it and then decide whether we want further code and docs changes in a future PR. I think it is safe to tag after merging this PR, because I am relatively confident that we can keep the current |
This assumes that context helper functions (go-logr#213) for slog will not get merged. If that is the consensus, then this documentation should be the last missing piece for slog support in logr.
1d7c75d to
6a76263
Compare
This assumes that context helper
functions (#213) for slog will not get merged. If that is the consensus, then this documentation should be the last missing piece for slog support in logr.
Fixes: #212