Skip to content

ref(replay): Enable no-console rule #6347

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

Merged
merged 1 commit into from
Nov 30, 2022
Merged

ref(replay): Enable no-console rule #6347

merged 1 commit into from
Nov 30, 2022

Conversation

mydea
Copy link
Member

@mydea mydea commented Nov 30, 2022

This enabled no-console eslint rule.

I left two cases of actual console.warn in, where it is basically a deprecation for old settings (which we can probably eventually delete).

@mydea mydea added the Package: replay Issues related to the Sentry Replay SDK label Nov 30, 2022
@mydea mydea requested a review from Lms24 November 30, 2022 14:37
@mydea mydea self-assigned this Nov 30, 2022
Comment on lines 182 to 193
if (usingDeprecatedCaptureOnlyOnError) {
// eslint-disable-next-line no-console
console.warn(
'[@sentry/replay]: The `captureOnlyOnError` option is deprecated! Please configure `errorSampleRate` instead.',
);
}

if (usingDeprecatedReplaysSamplingRate) {
// eslint-disable-next-line no-console
console.warn(
'[@sentry/replay]: The `replaysSamplingRate` option is deprecated! Please configure `sessionSampleRate` instead.',
);
Copy link
Member

@Lms24 Lms24 Nov 30, 2022

Choose a reason for hiding this comment

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

I think the change is good for now but WDYT, should we think about removing these deprecated options? We're still in alpha and soon bumping to 7.x so it might be wort a thought.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I would remove them in a separate PR. I think it makes sense to put this into the 0.x --> 7.x changelog, as people will be most likely to actually read that!

Copy link
Member

@Lms24 Lms24 Nov 30, 2022

Choose a reason for hiding this comment

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

Yes, separate PR sounds good to me. Let's just make sure to keep the Replay team informed about it.

@github-actions
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.57 KB (-0.02% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 60.59 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.36 KB (+0.02% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 54.15 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 20.12 KB (0%)
@sentry/browser - Webpack (minified) 65.75 KB (0%)
@sentry/react - Webpack (gzipped + minified) 20.14 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 45.99 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.45 KB (-0.01% 🔽)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 24.88 KB (+0.02% 🔺)
@sentry/replay index.js 62.08 KB (+0.01% 🔺)
@sentry/replay - Webpack (gzipped + minified) 62.08 KB (-0.01% 🔽)
@sentry/replay - Webpack (minified) 199.88 KB (+0.03% 🔺)

@mydea mydea merged commit 14e5e48 into master Nov 30, 2022
@mydea mydea deleted the fn/replay-no-console branch November 30, 2022 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: replay Issues related to the Sentry Replay SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants