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

Return Result from vm_memory.rs methods instead of using expect #65

Closed
cptartur opened this issue Jul 6, 2023 · 5 comments
Closed

Return Result from vm_memory.rs methods instead of using expect #65

cptartur opened this issue Jul 6, 2023 · 5 comments
Labels

Comments

@cptartur
Copy link
Member

cptartur commented Jul 6, 2023

software-mansion/protostar#2006

There are methods in vm_memory.rs using expect. These should be changed to return anyhow result with relevant message. It should panic at the cheatcodes_hint_processor level, or this method should be moved there.

@cptartur cptartur added this to the Forge milestone Jul 6, 2023
@piotmag769
Copy link
Member

Is it still relevant?

@cptartur
Copy link
Member Author

Not sure, someone could take this issue and verify it it makes sense still.

@MaksymilianDemitraszek
Copy link
Member

@cptartur Please include specific code locations when creating such issues, otherwise it is just: Check the whole codebase and see if we can do something better which is not very productive :p

@piotmag769
Copy link
Member

I think this task was created when the runner was still changing very rapidly, so it made sense back then ;P

@cptartur
Copy link
Member Author

I've verified and it seems only methods that still have unwrap or expect, aside from parts of cheatcodes hint processor, are on vm_memory.rs.

Updating the issue description

@cptartur cptartur changed the title Implement better error handling Return Result from vm_memory.rs methods instead of using expect Jul 25, 2023
@cptartur cptartur reopened this Jul 25, 2023
@cptartur cptartur closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2023
@MaksymilianDemitraszek MaksymilianDemitraszek removed this from the Forge milestone Jul 26, 2023
Utilitycoder pushed a commit to Utilitycoder/starknet-foundry that referenced this issue Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

3 participants