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

TOB-DQK-007 remove panics #95

Merged
merged 21 commits into from Jan 31, 2022
Merged

TOB-DQK-007 remove panics #95

merged 21 commits into from Jan 31, 2022

Conversation

MarioDfinity
Copy link
Contributor

I removed the (panic|unwrap|expect)s from the project on bad user inputs and similar errors that are expected to happen and should be reported with a human friendly message. Some of my changes are redundant, e.g. the unwrap on opts that get also validated by validators, but the new code is not much more than unwrapping.

I also added some tests in main.rs that helped me validating my changes to that file.

@roman-kashitsyn
Copy link

Great work! I think we can improve it further by using context or with_context feature of anyhow as much as possible. These functions wrap the errors to form a chain, which makes it possible for us to be more creative and helpful when we display the error.

src/commands/generate.rs Outdated Show resolved Hide resolved
src/commands/generate.rs Outdated Show resolved Hide resolved
src/commands/mod.rs Outdated Show resolved Hide resolved
src/commands/neuron_manage.rs Outdated Show resolved Hide resolved
src/commands/neuron_manage.rs Outdated Show resolved Hide resolved
src/lib/mod.rs Outdated Show resolved Hide resolved
src/lib/mod.rs Outdated Show resolved Hide resolved
src/lib/mod.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
MarioDfinity and others added 8 commits January 28, 2022 14:05
Co-authored-by: Roman Kashitsyn <roman@dfinity.org>
Co-authored-by: Roman Kashitsyn <roman@dfinity.org>
Co-authored-by: Roman Kashitsyn <roman@dfinity.org>
Co-authored-by: Roman Kashitsyn <roman@dfinity.org>
Co-authored-by: Roman Kashitsyn <roman@dfinity.org>
Co-authored-by: Roman Kashitsyn <roman@dfinity.org>
Co-authored-by: Roman Kashitsyn <roman@dfinity.org>
Co-authored-by: Roman Kashitsyn <roman@dfinity.org>
@MarioDfinity
Copy link
Contributor Author

The build is going to fail because files are not importing anyhow::Context. I'll fix it myself in a moment.

@MarioDfinity
Copy link
Contributor Author

@roman-kashitsyn I've changed most of the code to use context instead of map_err when it makes sense. map_err is now used only to wrap an error not supported by anyhow inside an anyhow error. Please have a look again.

@MarioDfinity MarioDfinity merged commit 310d7d6 into master Jan 31, 2022
@MarioDfinity MarioDfinity deleted the TOB-DQK-007_remove_panics branch January 31, 2022 09:03
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.

None yet

2 participants