Replace the multierror dependency with a simple error aggregator. #207
Conversation
| } | ||
| } | ||
| tf.files = nil | ||
| return nil |
There was a problem hiding this comment.
pretty sure this was a bug 😅
mattmoor
force-pushed
the
multi-error
branch
from
701c271
to
f4fc3bc
Compare
estargz/errors.go
Outdated
estargz/build.go
Outdated
| } | ||
| } | ||
| tf.files = nil | ||
| return nil | ||
| return allErr |
There was a problem hiding this comment.
We should return nil when no error is appended to allErr?
There was a problem hiding this comment.
if nothing is ever appended to it, it is still nil
There was a problem hiding this comment.
🤔 or you are saying this is the weird Go typed nil nonsense. blah, let me see...
There was a problem hiding this comment.
I reworked this ~completely to just use []error directly and have the errorutil package expose an Aggregate method for combining them. I think this ends up being a lot simpler, and it avoid the typed nil case.
mattmoor
force-pushed
the
multi-error
branch
from
a10ff77
to
5b24ccd
Compare
estargz/errorutil/errors.go
Outdated
| return strings.Join(points, "\n\t") | ||
| } | ||
|
|
||
| var _ error = (*Errors)(nil) |
There was a problem hiding this comment.
We have tests, so we do not need this line
There was a problem hiding this comment.
I reworked the change pretty dramatically, so I'm no longer implementing a custom error type at all anyways.
|
Please squash commits |
This lets functions that want to aggregate and return a collection of errors use a `[]error` to accumulate them, and `errorutil.Aggregate(list)` to turn them into a single `error`. If the list is empty, the `error` will be nil. If the list is a singleton, it will return the single error. Otherwise, it will construct a new error combining the constituent error messages. Fixes: containerd#205 Signed-off-by: Matt Moore <mattmoor@vmware.com>
mattmoor
force-pushed
the
multi-error
branch
from
5b24ccd
to
cf6d7a0
Compare
Given the overhaul I just gave this, I didn't even bother with a squash, since it's a completely different change. I'll update the PR subject and description above now to reflect the new commit message. |
mattmoor
changed the title
|
Thanks for the quick review, and thanks again for driving this super important work! 🤩 |
This lets functions that want to aggregate and return a collection of errors use a
[]errorto accumulate them, anderrorutil.Aggregate(list)to turn them into a singleerror. If the list is empty, theerrorwill be nil. If the list is a singleton, it will return the single error. Otherwise, it will construct a new error combining the constituent error messages.Fixes: #205