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

panic cleanup #153

Closed
garious opened this issue Apr 26, 2018 · 4 comments
Closed

panic cleanup #153

garious opened this issue Apr 26, 2018 · 4 comments
Labels
good first issue Good for newcomers

Comments

@garious
Copy link
Contributor

garious commented Apr 26, 2018

We're too cavalier with calls to expect() and unwrap(). We're using them safely for the RwLocks, but most others should be acknowledged or added to the return values.

@garious garious added the good first issue Good for newcomers label Apr 27, 2018
@jackson-sandland
Copy link
Contributor

jackson-sandland commented May 9, 2018

For the unsafe calls to expect(), is a panic still acceptable on err as long as there are good error messages? Since multiple calls to unwrap() will print the same panic message, doesn't it make more sense to use expect(), instead of unwrap(), with an error message detailed enough to indicate where the panic occurred?

@garious
Copy link
Contributor Author

garious commented May 9, 2018

I think replacing every call to unwrap() with expect("something unique") would be a first good step.

@jackson-sandland
Copy link
Contributor

jackson-sandland commented May 9, 2018

Sounds good. I'll start there. I'll leave off replacing calls to unwrap() in tests / benchmarking.

garious added a commit that referenced this issue May 11, 2018
@garious
Copy link
Contributor Author

garious commented May 11, 2018

Thanks @jackson-sandland!

@garious garious closed this as completed May 11, 2018
35359595 pushed a commit to 35359595/solana that referenced this issue Mar 23, 2024
* frozen-abi-macro: use `log` from `solana_frozen_abi`

* use private module approach
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants