Skip to content
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(jumpstart): correct jumpstart contracts defaultValue #4923

Merged
merged 4 commits into from
Feb 16, 2024

Conversation

bakoushin
Copy link
Contributor

Description

  1. All possible dynamic config properties must be listed in defaultValues when declaring that config.
    Otherwise, they won't be forwarded.

  2. In addition, it seems more appropriate to use NetworkId as network specifier instead of Network to enable jumpstart contracts on the testnet as well.

Test plan

  • Updated unit tests

Related issues

Backwards compatibility

Jumpstart functionality is not fully released yet

Network scalability

If a new NetworkId and/or Network are added in the future, the changes in this PR will:

  • Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)

@bakoushin bakoushin changed the title fix(jumpstart): correct jumstart contracts defaultValue fix(jumpstart): correct jumpstart contracts defaultValue Feb 16, 2024
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (81e998c) 85.25% compared to head (2adc75b) 85.25%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4923      +/-   ##
==========================================
- Coverage   85.25%   85.25%   -0.01%     
==========================================
  Files         712      712              
  Lines       29230    29229       -1     
  Branches     5088     5088              
==========================================
- Hits        24920    24919       -1     
  Misses       4067     4067              
  Partials      243      243              
Files Coverage Δ
src/jumpstart/jumpstartLinkHandler.ts 92.59% <100.00%> (-0.14%) ⬇️
src/statsig/constants.ts 100.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81e998c...2adc75b. Read the comment docs.

@bakoushin bakoushin enabled auto-merge (squash) February 16, 2024 13:54
@bakoushin bakoushin merged commit 5d70e8e into main Feb 16, 2024
16 checks passed
@bakoushin bakoushin deleted the alex/fix-jumstart-contracts-dynamic-config branch February 16, 2024 20:33
shottah pushed a commit to zed-io/kolektivo that referenced this pull request May 15, 2024
…c#4923)

### Description

1) All possible dynamic config properties must be listed in
`defaultValues` when declaring that config.
Otherwise, they [won't be
forwarded](https://github.com/valora-inc/wallet/blob/a0b4675774579822a6c2ac55e7de38f6658b45c2/src/statsig/index.ts#L29-L34).

2) In addition, it seems more appropriate to use `NetworkId` as network
specifier instead of `Network` to enable jumpstart contracts on the
testnet as well.

### Test plan

* Updated unit tests

### Related issues

- Related to  RET-999

### Backwards compatibility

Jumpstart functionality is not fully released yet

### Network scalability

If a new NetworkId and/or Network are added in the future, the changes
in this PR will:

- [x] Continue to work without code changes, OR trigger a compilation
error (guaranteeing we find it when a new network is added)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants