Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

feat: add ProviderExt trait #1559

Merged
merged 1 commit into from
Aug 3, 2022

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Aug 3, 2022

Motivation

moved from foundry-rs/foundry#2559

Solution

  • add ProviderExt trait for HttpProvider

this is currently sealed to prevent impl outside of ethers until we finalize #1267

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@gakonst gakonst merged commit 3040edf into gakonst:master Aug 3, 2022
Chain::Oasis => 5_500,
Chain::Emerald => 6_000,
Chain::Dev | Chain::AnvilHardhat => 200,
_ => return None,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to explicitly catch other variants, otherwise it will be very easy for others contributor that are adding new networks to miss this match!

Copy link
Owner

Choose a reason for hiding this comment

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

What should we do? panic?

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry I wasn't clear. Still return None is fine, but instead of using _ as a pattern, write down all other enum variants.

This way if a new network is added, compiler will help developer noticing that this is something to fill in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reasonable, do you want to add this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants