Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(multichain): Use new dynamic config for multichain features #4265
feat(multichain): Use new dynamic config for multichain features #4265
Changes from 7 commits
16750ab
3390c5f
7de56fc
fcb1ec9
543e9e3
94c024c
620c4dc
2fc53e0
a4a0826
45430a6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Noticed these in a bunch of test files, are the mocks on these test files required because there's a nested dependency to this dynamic config? And is the getFeatureGate mock now required? I wonder if we should consider creating a shared mock under
__mocks__
similar to how navigate / analytics is mocked so we don't need this everywhereThere 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.
That's correct, re: nested dependency. And yes, the
getFeatureGate
mock is still required here. And interesting -- I haven't messed around with shared mocks before. How would that work? Would there be a mock for the entirety ofsrc/statsig
that would automatically be included in all tests? What if we want to override this mock (which will certainly be the case)?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.
yes it would mock
src/statsig
for all tests. We can still setup test specific mocks as we do normally and it will override this mock. In any case its outside the scope of this PR, but something to consider if we have more of these shared mocks.