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

fix: Do not panic on multiple install calls #114

Merged
merged 5 commits into from
Jul 29, 2022

Conversation

jaysonsantos
Copy link
Contributor

When writing tests it is really useful to use color-eyre to have better messages but, calling install multiple times will panic.
This should return an error instead.
It should fix #97

@jaysonsantos
Copy link
Contributor Author

I wonder if it would be better to use sync::Once on color_eyre::install to make sure only one instance is being installed using a singleton, what do you think? let me know and i can change the pr

@yaahc
Copy link
Collaborator

yaahc commented Jul 26, 2022

I think I prefer this version as is over doing it in a Once. It's easy enough1 to write a wrapping function using a Once if you're going to intentionally race a bunch of installs, and in my experience color-eyre isn't the only library where you need to worry about initializing it once in tests so leaving that to the user seems like the right balance to me.

Footnotes

  1. https://github.com/ZcashFoundation/zebra/blob/main/zebra-test/src/lib.rs#L62-L143

@jaysonsantos
Copy link
Contributor Author

@yaahc cool! Then, I just sent the fix for the example as I only ran a simple cargo test here

@jaysonsantos jaysonsantos requested a review from yaahc July 27, 2022 21:07
@yaahc
Copy link
Collaborator

yaahc commented Jul 27, 2022

I updated an error message and I think it's gonna break fmt :X

edit: it didn't! Easyyyyy

@jaysonsantos
Copy link
Contributor Author

this also seems to fix #78

@yaahc yaahc merged commit ad68589 into eyre-rs:master Jul 29, 2022
@akshayknarayan akshayknarayan mentioned this pull request Oct 25, 2022
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.

install call panics instead of returning a result
2 participants