Add thread-safe multi error implementation #2
Conversation
Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
| testutil.NotOk(t, func() error { | ||
| return e.Err() | ||
| }()) |
There was a problem hiding this comment.
Hm, can't this be
| testutil.NotOk(t, func() error { | |
| return e.Err() | |
| }()) | |
| testutil.NotOk(t, e.Err()) |
| @@ -19,12 +19,11 @@ | |||
| // | |||
| // Example 3: | |||
| // | |||
| // func CloseAll(closers []io.Closer) error { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
|
|
||
| // NilOrMultiSyncError is a thread-safe implementation of NilOrMultiError. | ||
| // It allows combining multiple errors into one. | ||
| type NilOrMultiSyncError struct { |
There was a problem hiding this comment.
I wonder if we don't want to make the normal implementation concurrency safe 🤔 I don't think it introduces worth noting overhead... The only risk is when somebody would like to have different synchronization techniques that works for them, then they would need to reimplement all.. but sounds like rare use case.
Otherwise another option is to have different package..
I think for me it would make more sense to have ONLY gorotoutine safe implementation here.
There was a problem hiding this comment.
I think it depends on what use cases there are out in the wild, but I'd agree having one implementation and documenting that it's concurrency safe so it's clear for the users is the way to go. I'll prepare the changes.
|
I would vote for having just concurrency safe code for I would do it (: cc @saswatamcode @matej-g |
There was a problem hiding this comment.
We have to revisit merror. https://pkg.go.dev/errors@master#Join entered and it might mean we can drop merror completely.
This is directly inspired by @fpetkovski's work in thanos-io/thanos#5420 (comment).
This PR adds a thread-safe implementation of the
NilOrMultiErrorcalledNilOrMultiSyncError. PR provides tests as well.Another small fix in this PR is to unify the method receivers
NilOrMultiErrorto use pointers.