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

Mark envOr functions as view #6757

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Conversation

anikaraghu
Copy link
Contributor

@anikaraghu anikaraghu commented Jan 10, 2024

Motivation

I tried using vm.envOr in a view function and noticed it wasn't marked as view, so this led to errors. Checked with foundry devs and looks like this was just an oversight.

Solution

Add view modifier to envOr functions

@@ -1060,15 +1060,14 @@ impl Backend {
env.block.basefee = base;
}

let gas_price =
gas_price.map(|g| g).or(max_fee_per_gas.map(|g| g)).unwrap_or_else(|| self.gas_price());
let gas_price = gas_price.or(max_fee_per_gas).unwrap_or_else(|| self.gas_price());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

clippy was complaining about this

@anikaraghu anikaraghu marked this pull request as ready for review January 11, 2024 04:28
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm!

this doesn't conflict with forge-std, right @DaniPopes @mds1 ? so we can merge this independently?

@mds1
Copy link
Collaborator

mds1 commented Jan 11, 2024

@mattsse correct, once this is merged we just need to run the forge-std python script to update forge-std, then I can put out a new release

@mattsse mattsse merged commit 69a9c14 into foundry-rs:master Jan 11, 2024
18 of 19 checks passed
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.

3 participants