-
Notifications
You must be signed in to change notification settings - Fork 158
replace hashicorp/go-multierror with errors.Join #778
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
base: master
Are you sure you want to change the base?
Conversation
|
Ah, right, this requires go1.20 as minimum, which perhaps by now may be fine? |
87d4560 to
73b898b
Compare
|
@thaJeztah #776 is merged; time to rebase |
73b898b to
7769afa
Compare
| url, err := JSONSchemaURL(strings.TrimSuffix(v.spec.Version, "-dev")) | ||
| if err != nil { | ||
| errs = multierror.Append(errs, err) | ||
| errs = errors.Join(errs, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to check if I'm actually doing these right; I recently made a mistake with stdlib "multi-errors" because (IIRC) errors.Join is more an equivalent to errors.Wrap, and nests a new error, not making it on the same "level" as other errors when using a loop.
|
With #777 we're switching to go 1.21 as a minimum so once merged we can merge this one as well. |
Signed-off-by: Sebastiaan van Stijn <[email protected]>
7769afa to
ad4ac5c
Compare
|
Rebased after #777 was merged |
|
@kolyshkin @tianon PTAL |
|
Wonder if we can actually test it (or is it tested already). If yes, I'm all for it. |
kolyshkin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
I think https://github.com/opencontainers/runtime-tools/graphs/contributors tells a pretty clear story about whether or not anyone is actively maintaining the code in this repository in 2025 (and frankly even as far back as 2019), and I'd honestly suggest that any project finding value from it should probably be thinking about either forking or adapting to some new library, if there's some code here that's still useful. Is there a particular reason you're looking at this, Seb? Some transitive dependency pulling this into a project you maintain?
Like honestly, this description is really |
|
Oh! I actually forgot my own comment to have another look at "wrap" vs "same level"; #778 (comment)
Yeah, I recalled this PR when I updated this in containerd (containerd/containerd#12320) to get rid of some transitive dependencies (including the unmaintained
Yeah, I fully agree; this project is not really maintained, so any user of it may expect specs generated by it to be outdated and/or missing changes from current OCI specs. I think for most it's just as a convenience to either generate an OCI spec in (integration) tests, but for others it looks to be used a part of their core functionality; at least these come to mind;
For containerd ... it's actually slightly surprising, because it's depending on runtime-tools to implement many things that containerd itself also implements (through its own |
|
FWIW, we do use I finally managed to remove the
For The validation stuff in this repo was meant to end up tying into compliance but that never really got off the ground AFAIK. |
Uh oh!
There was an error while loading. Please reload this page.