Skip to content

Conversation

@stephhuynh18
Copy link
Contributor

No description provided.

@codecov-commenter
Copy link

Codecov Report

Base: 85.27% // Head: 85.17% // Decreases project coverage by -0.09% ⚠️

Coverage data is based on head (ce97090) compared to base (011f667).
Patch coverage: 66.66% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #885      +/-   ##
===========================================
- Coverage    85.27%   85.17%   -0.10%     
===========================================
  Files           26       26              
  Lines         1297     1302       +5     
  Branches       190      194       +4     
===========================================
+ Hits          1106     1109       +3     
- Misses         191      193       +2     
Impacted Files Coverage Δ
...blockchain/ethereum/ethereum-blockchain-service.ts 90.41% <66.66%> (-0.94%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@v-stickykeys v-stickykeys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling here seems a bit redundant and hard to follow.

Can we simplify to just

If usesmartcontractanchors { if contractaddress {...} else { throw } }

@stephhuynh18
Copy link
Contributor Author

The error handling here seems a bit redundant and hard to follow.

Can we simplify to just

If usesmartcontractanchors { if contractaddress {...} else { throw } }

@v-stickykeys yes it is =( I was being lazy HAHA because of the tests dislikes the option you provided. But i should do it right.

@gvelez17
Copy link
Contributor

hey @stephhuynh18 should we fix conflicts and merge this during our on-call week? figured cleanup old branches

)
}

if (this.config.blockchain.connectors.ethereum.contractAddress) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this if since we have the throw above? but ya i like the clear error message

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.

5 participants