Codecov Report

Merging #1169 (68dcebb) into master (bb9cd1f) will decrease coverage by 0.02%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1169      +/-   ##
==========================================
- Coverage   10.85%   10.83%   -0.03%     
==========================================
  Files         154      154              
  Lines        5649     5660      +11     
  Branches     1792     1800       +8     
==========================================
  Hits          613      613              
- Misses       5034     5045      +11     
  Partials        2        2

Impacted Files	Coverage Δ
src/hooks/useApproveAndDeposit.ts	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

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

arthur-zhuk commented 2 years ago

Mostly minor feedback about doing null checks at the start of functions to avoid a lot of option chaining later on. Logic looks good.

Double check that you can correctly approve and deposit on localhost.

I'd also be curious to get your thoughts on how you'd refactor this file, since this is one of the oldest in the repo and could definitely be improved.

Approving to unblock.

I confirmed approval and deposit works on localhost.

approveSingleToken can be pulled out into a hook utility file. Gas logic can be deleted as it's not utilized. Some of the conditional statements could be refactored to be simpler. Deadline and approval logic can be refactored into a function and pulled out of the file. metaswapContract can be pulled out of the file and imported.

saddle-finance / saddle-frontend

useApproveAndDeposit POOLS_MAP removed #1169

Codecov Report