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

Migrate from sentry-raven to sentry-ruby #8878

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Migrate from sentry-raven to sentry-ruby #8878

merged 1 commit into from
Jan 25, 2024

Conversation

JamieMagee
Copy link
Contributor

@JamieMagee JamieMagee commented Jan 23, 2024 •
edited

Reverts #8874 (Cherry-picked 85251aa from #8818)

NOTE: Do not merge this yet.

Verified in staging. Ready to merge.

@JamieMagee JamieMagee requested a review from a team as a code owner January 23, 2024 22:12
@jeffwidman
Copy link
Member

Once you do figure out the problem, I'd be curious to read what it was...

@JamieMagee JamieMagee force-pushed the jamiemagee/sentry-ruby branch 5 times, most recently from 2fd0d78 to 35100dd Compare January 24, 2024 19:05
@JamieMagee
Copy link
Contributor Author

@jeffwidman It was two issues:

  1. I had a dependency cycle between the concrete classes ExceptionSanitizer/SentryContext and the abstract Processor class. Processor had an abstract process method, but also had the static process_chain which hardcoded the ExceptionSanitizer and SentryContext
  2. I forgot to call .new on ExceptionSanitizer and SentryContext. [ExceptionSanitizer, SentryContext] doesn't reference instances of the classes.

@JamieMagee JamieMagee force-pushed the jamiemagee/sentry-ruby branch 8 times, most recently from 1cf1e6d to 731cd46 Compare January 24, 2024 23:27
@JamieMagee JamieMagee mentioned this pull request Jan 24, 2024
Merged
@jakecoffman jakecoffman merged commit b2703ab into main Jan 25, 2024
118 of 119 checks passed
@jakecoffman jakecoffman deleted the jamiemagee/sentry-ruby branch January 25, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deivid-rodriguez deivid-rodriguez deivid-rodriguez approved these changes

Assignees
No one assigned
Labels
L: javascript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@JamieMagee @jeffwidman @deivid-rodriguez @jakecoffman