-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat: Switch to color_eyre and remove unneeded .unwrap()s #426
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Antosser! Thanks so much for this PR.
Can you please split concerns in two PRs please?
unwrap
removalscolor_eyre
(this needs extra discussion)
Lets focus on merge 1 and then revisit 2 once 1 is merged!
|
I rather focusing in each topic, makes PRs easier to review and also development in general easier to follow. Given that we are writing this for the community, these kind of decisions needs some discussion on Can we focus in |
Rewriting everything right now would be a lot of work, just to probably undo it later. Let's discuss it first, so I won't have to do as many changes |
I started a discussion for this: #427 |
Nice! Let's just remove unwraps in this PR please, so we can merge it! |
Wdym? |
As I've said, migrating back to |
I think I can help you. I will open a PR addressing #424. Then we merge and we can rebase against this. WDYT? |
To be specific, in this PR we are addressing 2 concerns that are strongly mixed. Each error message should be handled and must provide a clear path for the user to address Reviewing this PR correctly its highly time consuming and that could easily be tackled by maintaining each PR scoped to a single concern. |
sure
Yeah, it's a lot
How? The amount of changes will stay the same. Only difference is that the error-handling library will be the same as before. Migrating back would take me much more time than for you to review this |
This PR is a lot. I've completely swapped
anyhow
withcolor_eyre
, and replaced mostunwrap
s with?
. Such changes required changing multiple function signatures and struct fields.Now, the errors should look way better than they did before.
Examples
Before
After
Before
After
Also #425