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

Updated readme to reflect current implementation of logLevels #73

Merged
merged 1 commit into from
Sep 16, 2022

Conversation

wesbragagt
Copy link

Looks like the implementation of SentryModule.forRoot on version 6.17.4 changed on how logLevel is passed. Now it's spelled as logLevels and it receives a type of LogLevel[]. Also the type LogLevel doesn't seem to exist anymore. Documentation needs to be updated in order to avoid type conflicts.

Before:

SentryModule.forRoot({
...
logLevel: LogLevel.Debug
})

Now:

SentryModule.forRoot({
logLevels: ['debug']
})

@joao-felipe-santoro
Copy link

was about to do the same ! nice catch @wesbragagt !

@wesbragagt
Copy link
Author

Haha, no problem @joao-felipe-santoro. Hopefully that helps folks getting type issues when trying to build the project. I ran tsc --noEmit to find the actual name for the property.

@rubiin
Copy link

rubiin commented Apr 26, 2022

Why isn't this merged? seems to be a big issue when someone follows the readme

@ctsstc
Copy link

ctsstc commented Jun 1, 2022

I tried using Severity for LogLevel, but the typing complains even though they values resolve to the same values from what I can tell. The typing should be updated to use Severity even though it sounds like it's deprecated in the latest v7 release as mentioned here: #84

This ended up working although hacky and soon to be deprecated:

logLevels: [
  Severity.Fatal,
  Severity.Critical,
  Severity.Error,
  Severity.Warning,
] as LogLevel[]

PS: it's actually importing the LogLevel interface from: @nestjs/common, but it seems happy now lol.

Edit: It's still sending everything, so this doesn't seem to be honored :[
Looks like I'll have to do something like:#42 (comment)

@donprecious
Copy link

this pull request should be merged by now , since it important to new people using this library .

@wesbragagt
Copy link
Author

@ntegral this needs merged

@ntegral ntegral self-assigned this Sep 16, 2022
Copy link
Owner

@ntegral ntegral left a comment

Choose a reason for hiding this comment

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

This will correct the typo

@ntegral ntegral merged commit c431875 into ntegral:master Sep 16, 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.

6 participants