-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
chore: merge ScopeExtensions to Scope #3186
Conversation
Are we running the same build twice now after |
@@ -6,6 +6,10 @@ | |||
|
|||
- Added Crons support via `SentrySdk.CaptureCheckIn` and an integration with Hangfire ([#3128](https://github.com/getsentry/sentry-dotnet/pull/3128)) | |||
|
|||
### API changes | |||
|
|||
- Removed `ScopeExtensions` class - all the public methods moved directly to `Scope` ([#3186](https://github.com/getsentry/sentry-dotnet/pull/3186)) |
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.
Is that change even visible to users?
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.
Shouldn't be for c# users
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.
Their code won't have to change if they're recompiling with the new code. Technically I guess it's not binary compatible (if someone was using reflection to get at these extension methods for some strange reason, that would break)... I'm not sure how hard core we want to be about avoiding breaking changes. @bruno-garcia can we be "pragmatic" 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.
can we be "pragmatic" here?
Yes, especially since this is improving/fixing things on the Powershell side and not just a refactor for fun.
In PowerShell, extension classes are just standard classes with static methods, so you had to do
[Sentry.ScopeExtensions]::AddEventProcessor($scope, $processor)
with this PR, you can now:
$scope.AddEventProcessor($processor)