feat: add shared trackError util - #246
Conversation
4002b2b to
8fa2cf2
Compare
8fa2cf2 to
7c14039
Compare
7c14039 to
84b9966
Compare
mikesposito
left a comment
mikesposito
left a comment
There was a problem hiding this comment.
LGTM, just a small question
| export function getSnapProvider(): SnapsProvider { | ||
| // snap is a global variable provided by the Snap SDK | ||
| return snap; | ||
| } |
There was a problem hiding this comment.
nit: I'm wondering, do we really need this function to use a globally available variable?
There was a problem hiding this comment.
this is needed mainly for tests, to get this global var when it is called
There was a problem hiding this comment.
can't we mock the global variable instead?
There was a problem hiding this comment.
yes we can for sure, imo mock a function is easier, I saw this pattern in Stellar snap, but they use this as a global pattern all over the snap to not get the snap var explicitly in different places.
so if you think its cleaner to pass the global snap to the shared util I will change
There was a problem hiding this comment.
no need to block this PR, we can consider this as a discussion to follow-up on if this is a pattern already used elsewhere
2 participants
Explanation
Create shared trackError util
References
Checklist