Skip to content

Conversation

@crooks
Copy link

@crooks crooks commented Nov 15, 2021

This pull request adds an Atoi helper function. The function provides a conversion from the standard log-level string representations to the integer levels used in log-go.

Copy link
Member

@mattfarina mattfarina left a comment

Choose a reason for hiding this comment

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

What is the use case for this function? The levels aren't used as part of the interface. Can you provide some insight into how this would be used?

log.go Outdated

// Atoi returns the loglevel integer associated with a common loglevel
// string representation.
func Atoi(loglevelStr string) (level int, err error) {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of calling it Atoi can the function be named ParseLevel?

While the implementations of the levels are ints under the hood, the ints aren't really used as ints. Atoi is a common function name for a function that turns strings into ints. This is turning specific names into Levels.

With that in mind, can a new type be created for the Level and return that instead of an int.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Matt, thanks for the feedback. Regarding the use case for the function: My goal was to provide a means for translating the string-type loglevels usually specified in config files into the internal log-go loglevels. This could be done at the downstream implementation but, I think, the string representations of log levels are sufficiently standardised that the upstream library is the most logical place to host this function.

crooks added 2 commits July 2, 2022 10:47
Changes suggested by upstream maintainer Matt Farina.

Signed-off-by: Steve Crook <[email protected]>
@crooks crooks changed the title Add an Atoi funtion for loglevel string conversion Add a ParseLevel funtion for loglevel string conversion Jul 2, 2022
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