-
-
Notifications
You must be signed in to change notification settings - Fork 32
envelope has dedicated endpoint #353
envelope has dedicated endpoint #353
Conversation
this.sentryUrl = sentryUrl; | ||
|
||
try { | ||
final URI uri = sentryUrl.toURI(); |
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.
@bruno-garcia ideally I'd get this URI
in the ctor instead of the current URL
, but I'd not like to break the public API, I think this would be fine here.
what do you think?
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.
yeah this looks good for now but worth adding a task for v3.0.0 to refactor this. Perhaps we need to revisit the Dsn
type (like add a toStore
and toEnvelope
or something completely different).
Such discussion for sure woudl involve the whole SDK team
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.
We can go ahead with this but there's one comment we ideally would need to discuss
this.sentryUrl = sentryUrl; | ||
|
||
try { | ||
final URI uri = sentryUrl.toURI(); |
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.
yeah this looks good for now but worth adding a task for v3.0.0 to refactor this. Perhaps we need to revisit the Dsn
type (like add a toStore
and toEnvelope
or something completely different).
Such discussion for sure woudl involve the whole SDK team
yeah but |
📢 Type of change
📜 Description
envelope has a dedicated endpoint.
💡 Motivation and Context
@jan-auer can tell more about it.
💚 How did you test it?
fixed and ran unit tests.
📝 Checklist
🔮 Next steps