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

feat: added custom type for LogLevel #4

Closed
wants to merge 1 commit into from

Conversation

escarls
Copy link

@escarls escarls commented Nov 29, 2023

With this type you will get a slog.LogLevel from the variable and if the value is not recognized, it will just return slog.LevelInfo

@escarls escarls requested review from hedlund and engvik November 29, 2023 18:32
@engvik
Copy link
Member

engvik commented Nov 30, 2023

This is nice, as you pointed out, this requires Go 1.21, so technically it's a breaking change. Don't think we have any runtimes that don't support Go 1.21, but let's have a look first.

@audunmo
Copy link

audunmo commented May 16, 2024

Is this going to be merged?

@escarls escarls force-pushed the feat/add-slog-loglevel-support branch from 8271675 to 07ece39 Compare September 10, 2024 13:26
@escarls
Copy link
Author

escarls commented Sep 10, 2024

I've rebased this against the latest changes to main.
And it should be ok to add, as go versions lower than 1.21 is not currently supported anymore.

@engvik
Copy link
Member

engvik commented Sep 11, 2024

We should probably add the go directive to go.mod to indicate that this must be built with go1.21 and above.

With this type you will get a slog.LogLevel from the variable and
if the value is not recognized, it will just return slog.LevelInfo

Update the go versions matrix to at least 1.21,
as slog requires that version
@escarls escarls force-pushed the feat/add-slog-loglevel-support branch from 07ece39 to 1cd2cc3 Compare September 11, 2024 11:31
@engvik
Copy link
Member

engvik commented Sep 13, 2024

Sorry for the back and forth, I know that I was the one pointing you in this direction, but after thinking about it. It makes sense to add support for this in our internal logger package. Are you OK with that?

@escarls
Copy link
Author

escarls commented Sep 24, 2024

Closing this, as I've moved the code to cloud/logger

@escarls escarls closed this Sep 24, 2024
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.

3 participants