-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: core/browser additions #1175
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
Conversation
packages/core/__tests__/client.ts
Outdated
| const spy2 = jest.spyOn(sentry.getAdapter(), 'install'); | ||
| await sentry.install(); | ||
| await sentry.install(); | ||
| expect(spy1).toHaveBeenCalledTimes(2); |
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.
This assertion looks redundant, because you trigger those calls yourself 2 lines above
| const spy = jest.spyOn(sentry, 'setRelease'); | ||
| const spy2 = jest.spyOn(sentry.getAdapter(), 'setRelease'); | ||
| await sentry.setRelease('#oops'); | ||
| expect(spy).toBeCalled(); |
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.
Same here
|
|
||
| public setUserContext(user?: IUser) { | ||
| public getContext() { | ||
| // TODO: check for cyclic objects |
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.
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.
Thanks for the review, so about this function. I wasn't able to import the existing implementation and I want to have a fancy typescript and also thinking about if it would be possible to prevent circular refs upon setting it instead of reading.
No description provided.