Skip to content

Add threshold levels to forge snapshot --check #2509

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

Closed
devtooligan opened this issue Jul 29, 2022 · 1 comment
Closed

Add threshold levels to forge snapshot --check #2509

devtooligan opened this issue Jul 29, 2022 · 1 comment
Labels
T-feature Type: feature

Comments

@devtooligan
Copy link

Component

Forge

Describe the feature you would like

We thought adding forge snapshot --check to our CI was a great idea. It forces the dev to acknowledge and review any changes to the gas profile. But it didn't turn out to be that helpful. Pretty much every change affects the gas profile a little. So what ended up happening was that devs never reviewed it, and it just became an annoyance solved by quickly running forge snapshot and pushing a new commit without even looking at it.

One idea to improve this feature might be to add some sort of thresholds. For example, in foundry.toml user selects the minimum change (nominal or percentage) that must be met before the snapshot check fails. example:

// foundry.toml
minimum_snapshot_gas_increase_percent=5
minimum_snapshot_gas_increase_value=500
minimum_snapshot_gas_decrease_percent=5
minimum_snapshot_gas_decrease_value=1000

Anyways, open to other ideas on how to solve the problem. This is something that was mentioned on Twitter and gakonst asked for a ticket.

Additional context

No response

@devtooligan devtooligan added the T-feature Type: feature label Jul 29, 2022
@gakonst gakonst added this to Foundry Jul 29, 2022
@gakonst gakonst moved this to Todo in Foundry Jul 29, 2022
@mattsse
Copy link
Member

mattsse commented Jul 29, 2022

thanks, you already have an honorable mention here #2484 lol

+1 on making this a config option

closing this here as this is a dup of #2484

trying to get this over the line shortly.

@mattsse mattsse closed this as completed Jul 29, 2022
Repository owner moved this from Todo to Done in Foundry Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-feature Type: feature
Projects
Archived in project
Development

No branches or pull requests

2 participants