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

Add support for defining maximum log level #72

Merged
merged 4 commits into from
Oct 5, 2021

Conversation

jsynacek
Copy link
Contributor

@jsynacek jsynacek commented Jul 1, 2021

No description provided.

@jsynacek jsynacek marked this pull request as draft July 1, 2021 12:44
@@ -40,6 +41,9 @@ showLogLevel LogAttention = "attention"
showLogLevel LogInfo = "info"
showLogLevel LogTrace = "trace"

defaultLogLevel :: LogLevel
defaultLogLevel = LogTrace
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should probably be LogInfo.

@jsynacek jsynacek requested a review from arybczak July 23, 2021 14:12
@@ -32,6 +32,7 @@ data LoggerEnv = LoggerEnv
, leDomain :: ![T.Text] -- ^ Current application domain.
, leData :: ![A.Pair] -- ^ Additional data to be merged with the log
-- message\'s data.
, leMaxLogLevel :: LogLevel -- ^ The maximum log level to be logged.
Copy link
Contributor Author

@jsynacek jsynacek Jul 23, 2021

Choose a reason for hiding this comment

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

To make things very clear that it's the maximum (not minimum) allowed level, we should probably add a note to where LogLevel is defined and explicitly mention that it derives Ord. And the order in which the constructors are defined then implies which one is actually of "the highest value" and also the most verbose.

@jsynacek jsynacek marked this pull request as ready for review July 23, 2021 14:18
@jsynacek jsynacek force-pushed the dev-jsynacek-core3483-max-loglevel branch from 158c1fc to 2d90447 Compare August 16, 2021 08:22
@@ -40,6 +44,9 @@ showLogLevel LogAttention = "attention"
showLogLevel LogInfo = "info"
showLogLevel LogTrace = "trace"

defaultLogLevel :: LogLevel
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can add a haddock comment that it's LogInfo.

@arybczak arybczak merged commit dedf61c into master Oct 5, 2021
@jsynacek jsynacek deleted the dev-jsynacek-core3483-max-loglevel branch October 11, 2021 10:54
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