-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(js): Re-org js integrations and add missing node integrations #11799
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Bundle ReportChanges will increase total bundle size by 1.25kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
|
|
||
| ```JavaScript | ||
| Sentry.init({ | ||
| dsn: "___PUBLIC_DSN___", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would honestly not add the DSN here (and all the other snippets). It is not relevant to the snippet and it will create an ugly banner on top of the snippet if you are not logged in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(It also looks like you left it out in some places. Should we do all or nothing? I'd go for "nothing")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah let's go for nothing - I agree!
| ## Supported Versions | ||
|
|
||
| - Connect `^3.0.0` | ||
| - `connect`: `^3.0.0` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No init snippet here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I need to add them for all the server frameworks, just missed this
lforst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good cleanup! Mild concerns about consistency but nothing blocking.
ref getsentry/sentry-javascript#13317
Sentry.initsnippets to all relevant node integrationsdataloader,knex,andlrumemorizerintegrations