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

feat: Public methods now return MultiError instead of []error #123

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thrawn01
Copy link
Contributor

@thrawn01 thrawn01 commented May 30, 2023

Purpose

The current code base returns multiple errors which is inconvenient to use if multiple errors are not useful to the caller. Instead this PR creates an error that implements the Unwrap() []error method which can be used to retrieve multiple errors. This interface was introduced in go 1.20

Implementation

  • Added type MultiError struct
  • Added errorMsg() and errorMsgf() (Although that could be narrowed down to just errorMsg() with the capability to handle optional formatted text)

Versioning

This PR significantly breaks compatibility of the public interfaces. If this PR is acceptable, we should bump to v1 before merging.

Postscript

My editor naturally indents with a TAB, and as this project uses SPACES for indentation this PR mixes TABS and SPACES. If this PR is to be accepted I'll fix accordingly.

Reference: #117

@daveshanley
Copy link
Member

Thank you very much for doing this work. I know the change is small, but as you correctly determined, it's a large breaking change for existing consumers - however, it's the right change to make.

We will keep this PR warm until we're ready for 1.0, it should stay healthy and rebase easily when it comes time.

Thank you for your contribution.

(The build is failing btw :-)

@thrawn01 thrawn01 force-pushed the thrawn/multierror branch 2 times, most recently from 6bac51a to a5df7f4 Compare May 31, 2023 16:41
@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Patch coverage: 97.01% and no project coverage change.

Comparison is base (a4b7a01) 99.77% compared to head (7c8dcb0) 99.77%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #123   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files         146      147    +1     
  Lines       10174    10198   +24     
=======================================
+ Hits        10151    10175   +24     
  Misses         23       23           
Flag Coverage Δ
unittests 99.77% <97.01%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
document.go 97.95% <94.73%> (-0.10%) ⬇️
error.go 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@n0rig
Copy link

n0rig commented Nov 14, 2023

@thrawn01 do you have time to address the build issues and conflicts?

@thrawn01
Copy link
Contributor Author

I unfortunately do not, not until the end of the year Feel free to close or take the code as is if you want to use this code as the basis for a new PR.

No need to credit me.

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.

3 participants