-
Notifications
You must be signed in to change notification settings - Fork 213
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
ref: Use a Context type mapping to map[string]interface{} #444
Conversation
49650f9
to
f555dc7
Compare
.pre-commit-config.yaml
Outdated
- id: go-fmt | ||
- id: go-imports | ||
- id: golangci-lint | ||
- id: no-go-testing |
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 you move this to a separate PR?
interfaces.go
Outdated
@@ -161,10 +161,12 @@ type Exception struct { | |||
// An EventID must be 32 characters long, lowercase and not have any dashes. | |||
type EventID string | |||
|
|||
type Context map[string]interface{} |
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'm not sure if we should introduce a concrete Context
type. It'll force people to pass sentry.Context
explicitly. Where if we'd use type alias, type Context = map[string]interface{}
, it'd still work with most cases where people already pass a valid dictionary.
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 defined a new type since you mention on Slack it was a v0 and we should strive to make the API as it should. The type alias works too and the difference is minimal so that'll works well too.
Co-authored-by: Kamil Ogórek <kamil@sentry.io>
When Sentry processes an event, it only accepts a context being a key/value object. This PR aims to enforce we're passing a
map[string]interface{}
instead of aninterface{}
.