Skip to content

log: allow SetLevel to accept Level values - #22

Open
thaJeztah wants to merge 1 commit into
containerd:mainfrom
thaJeztah:better_level
Open

log: allow SetLevel to accept Level values#22
thaJeztah wants to merge 1 commit into
containerd:mainfrom
thaJeztah:better_level

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

Make SetLevel generic so callers can pass either a string or a Level directly.

Keep the existing string parsing behavior and global logger level synchronization unchanged.

@thaJeztah
thaJeztah force-pushed the better_level branch 4 times, most recently from c96f66d to b098eaa Compare August 14, 2026 09:06
@thaJeztah
thaJeztah marked this pull request as ready for review August 14, 2026 09:09
Comment thread context.go
return err
//
// SetLevel returns an error if a string level is not supported.
func SetLevel[T levelValue](level T) error {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

My only concern is that now it's impossible to assign this to a function-value without instantiation.

So if someone depends on passing log.SetLevel anywhere - it will fail now

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, can probably still pass it as log.SetLevel[string].

I doubt it's used like that, but I can check if there's any cases where it'd break (but also, ultimately, this whole module was meant to be for transitioning, so it should only be relevant to existing uses in containerd, moby 🤔)

Make SetLevel generic so callers can pass either a string or a Level
directly.

Keep the existing string parsing behavior and global logger level
synchronization unchanged.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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.

2 participants