-
Notifications
You must be signed in to change notification settings - Fork 18
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
➖ Removes redux-logger and makes use of custom logger middleware #47
Conversation
66af839
to
42b10c5
Compare
/snapit |
42b10c5
to
29184fb
Compare
🫰✨ Thanks @QuintonC! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/connect-wallet@0.0.0-snapshot-20230303202422 yarn add @shopify/gate-context-client@0.0.0-snapshot-20230303202422 yarn add @shopify/tokengate@0.0.0-snapshot-20230303202422 |
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.
👍
91a5609
to
2bc5b51
Compare
console.groupEnd(); | ||
} catch (error) { | ||
console.log('—— log end ——'); | ||
} |
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 looks much simpler than what was there before. I'm surprised that the tests didn't break.
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.
Haha what makes you think the tests would've broken? Previously we were just pushing logs into an array that would only ever have a single entry (from what I can tell), so it makes sense to me that it wouldn't have broken.
ℹ️ What is the context for these changes?
Removes
redux-logger
from the package and utilizes our own custom logger middleware. The source for this implementation is a simplified and slimmed down version of theredux-logger
code that not only matches the types of our store, but also one that we have more control over.🕹️ Demonstration
🎩 How can this be tophatted?
✅ Checklist
Tested on mobileN/ATested on multiple browsersN/ATested for accessibilityN/AUpdated relevant documentation for the changes (if necessary)N/A