-
Notifications
You must be signed in to change notification settings - Fork 96
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
fix: default context for painted door experiment #218
fix: default context for painted door experiment #218
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #218 +/- ##
=======================================
Coverage 96.46% 96.46%
=======================================
Files 195 195
Lines 1839 1839
Branches 322 322
=======================================
Hits 1774 1774
Misses 60 60
Partials 5 5 ☔ View full report in Codecov by Sentry. |
src/widgets/RecommendationsPaintedDoorBtn/PaintedDoorExperimentContext.jsx
Outdated
Show resolved
Hide resolved
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 tested this locally and it runs fine. Assuming Cindy's comment gets addressed, this LGTM!
aa34eee
to
0a57d8d
Compare
0a57d8d
to
b2f5a65
Compare
I'll be bringing this to our next team refinement session. |
b2f5a65
to
98e0742
Compare
@justinhynes, any objections to this? |
Hey @arbrandes , since I'm on-call this week, I will approve this now that I understand that it wasn't meant to be merged alongside #214 ! |
There are multiple places where attributes of the object provided by `usePaintedDoorExperimentContext()` are assumed to exist. This provides default (null) values for those attributes when creating the context.
@jsnwesson i just rebased this so it should be up to date now |
98e0742
to
0cedeb0
Compare
There are multiple places where properties of the object provided by
usePaintedDoorExperimentContext()
are assumed to exist. This provides default (null) values for those properties when creating the context.This PR is proposing a solution to #210 that would make it so #214 isn't needed.