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.

Sign up for GitHub

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

Jump to bottom

Stop using Severity enum #431

Merged
merged 11 commits into from
Jul 7, 2022
Merged

Stop using Severity enum #431

merged 11 commits into from
Jul 7, 2022

Conversation

JavierMartinz
Copy link
Contributor

@spawnia spawnia marked this pull request as draft May 30, 2022 17:51
@spawnia
Copy link
Collaborator

spawnia commented May 30, 2022

Please validate your changes locally and make sure CI passes - see https://github.com/DiederikvandenB/apollo-link-sentry/blob/master/CONTRIBUTING.md

@JavierMartinz JavierMartinz mentioned this pull request Jun 1, 2022
Closed
@danyonedwards
Copy link

Hey, just to help move this PR along. Looks the migration guide expects you to use the string value now not the enum.
https://github.com/getsentry/sentry-javascript/blob/master/MIGRATION.md#severity-severitylevel-and-severitylevels

E.G.
SeverityLevel.Error should just be the string literal 'error' now.

@JavierMartinz
Copy link
Contributor Author

JavierMartinz commented Jun 10, 2022 •
edited
Loading

Yeah, that's why I'm using SeverityLevel, because SeverityLevel.Error === 'error'.

The issue here is we need to bump @sentry/browser dependency.

@spawnia
Copy link
Collaborator

spawnia commented Jun 10, 2022

You can make whatever changes are required to make this pull request pass.

@JavierMartinz JavierMartinz marked this pull request as ready for review June 10, 2022 07:42
spawnia
spawnia requested changes Jun 10, 2022
View reviewed changes
src/SentryLink.ts Outdated Show resolved Hide resolved
src/SentryLink.ts Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
tests/SentryLink.test.ts Outdated Show resolved Hide resolved
@spawnia
Copy link
Collaborator

spawnia commented Jun 10, 2022

See failing test in https://github.com/DiederikvandenB/apollo-link-sentry/runs/6830041476?check_suite_focus=true#step:5:41

@spawnia spawnia marked this pull request as draft June 10, 2022 14:08
@JavierMartinz
Copy link
Contributor Author

Yeah, that test is failing because of the options type, which is inherited from https://github.com/wix/sentry-testkit and it looks like they didn't add support for Sentry 7 yet.

@spawnia
Copy link
Collaborator

spawnia commented Jun 13, 2022

Are we stuck waiting for zivl/sentry-testkit#108 or can we work around this issue?

Repository owner deleted a comment from FarooqQaisrani Jun 22, 2022
@JavierMartinz
Copy link
Contributor Author

I think we can wait for zivl/sentry-testkit#109

@JavierMartinz JavierMartinz marked this pull request as ready for review July 7, 2022 11:48
@spawnia spawnia merged commit e758576 into DiederikvandenB:master Jul 7, 2022
github-actions bot pushed a commit that referenced this pull request Jul 7, 2022
@semantic-release-bot
chore(release): 3.2.0 [skip ci]
a3438b5 
# [3.2.0](v3.1.3...v3.2.0) (2022-07-07)

### Features

* use `SeverityLevel` over `Severity` enum ([#431](#431)) ([e758576](e758576))
@github-actions
Copy link

github-actions bot commented Jul 7, 2022

🎉 This PR is included in version 3.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spawnia spawnia Awaiting requested review from spawnia

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JavierMartinz @spawnia @danyonedwards