Skip to content

Conversation

zenhack
Copy link
Contributor

@zenhack zenhack commented Jun 25, 2023

There is much more to do, but this is the beginning of 3PH.

@zenhack zenhack marked this pull request as draft June 25, 2023 22:41
zenhack added 24 commits June 28, 2023 23:47
Fails currently; the exception returned is 'failed' rather than
'unimplemented', which suggests that it may not be being forwarded
correctly. Need to investigate further.

This also factors out a bunch of common logic into a helper function.
This gets rid of a panic("TODO: ...").

The test still fails, but I know why.
That is, don't, and clean up.

For good measure, move the snapshot.Release() out of the critical
section, as I'm not sure this is valid.
...did I really not run the tests after this?
Still fails because of the obvious panic("TODO..."), but caught a couple
unrelated bugs.
The new interface:

- Has levels (debug, info, warn, error)
- Accepts arguments for structured logging

Per the comments, the interface is designed to be used with
*slog.Logger, though other implementations are possible (and we already
use one in test).
I'm going to punt this off until after the basic 3PH, so let's make sure
this is correct after that.
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.

1 participant