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

Refactor error handling #40

Merged
merged 2 commits into from
Jan 27, 2022
Merged

Refactor error handling #40

merged 2 commits into from
Jan 27, 2022

Conversation

oltrep
Copy link
Contributor

@oltrep oltrep commented Jan 25, 2022

  • create an error enum that implements Display
  • replace unwraps by wrapping errors into a ForemanError
  • refactor and simplify fs helpers to work with ForemanError

I had to touch a lot of code, but it's mostly trivial changes because it just adds map_err calls and change Result type annotations on functions.

* create an error enum that implements Display
* replace unwraps by wrapping errors into a ForemanError
* refactor and simplify fs helpers to work with ForemanError
@github-actions
Copy link

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

Copy link
Contributor

@ZoteTheMighty ZoteTheMighty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks solid to me! We might consider using an error library at some point, but this should be well equipped to migrate to it if we do.

Copy link
Contributor

@matthargett matthargett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests LGTM!

@oltrep oltrep merged commit c0eece4 into master Jan 27, 2022
@oltrep oltrep deleted the error-handling-refactor branch January 27, 2022 14:56
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 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.

3 participants