-
Notifications
You must be signed in to change notification settings - Fork 318
Bug/11196 conversion tracking snapshoting #11485
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
base: develop
Are you sure you want to change the base?
Bug/11196 conversion tracking snapshoting #11485
Conversation
…state should be added to the snapshot.
…conversion tracking data.
Build files for 9cd400c are ready:
|
Size Change: +1.22 kB (+0.06%) Total Size: 1.98 MB ℹ️ View Unchanged
|
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.
LGTM
…on-tracking-snapshoting.
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.
Can you add a QA brief for this issue so QA knows what to test here?
The code itself seems fine, though I think we can improve on the argument name.
* @return {Object} The snapshot store object. | ||
*/ | ||
export function createSnapshotStore( storeName ) { | ||
export function createSnapshotStore( storeName, { pick: pickPaths } = {} ) { |
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.
Rather than using pick
/pickPaths
, can we have a more descriptive name for this argument? Calling it pick
seems like the implementation detail of using lodash
's pick()
is bubbling up.
I'd call this keysToSnapshot
or onlySnapshotKeys
, etc. so it's a bit more clear to someone reading it.
Summary
Addresses issue:
Relevant technical choices
pick
as a string or an array of strings that are passed to lodash'spick
function.PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist