Make history field optional to match OCI spec#288
Conversation
Signed-off-by: Callum Groeger <cgroeger@amazon.com>
|
LGTM! Ship it 🚢 . This is a breaking change. |
utam0k
left a comment
There was a problem hiding this comment.
Thanks for your first contribution! LGTM
|
Great, thanks for approving. Just wondering when a release is likely to be made, as I'd like to have this fix out there. |
|
@groegercesg cc: @saschagrunert |
We should have bumped semver then, right? |
You're right, but it's already been released. |
|
You're right, but it's already been released.
The release can be yanked: https://doc.rust-lang.org/cargo/commands/cargo-yank.html
|
|
@cgwalters Thanks for your information. But I don't think it is worth yanking the current release. |
|
I know it can sometimes feel like a thankless task maintaining FOSS and only have people show up to complain. However...I have contributed to this repo in the past and am likely to again. I also use this crate in multiple projects, and this issue is causing friction in them. Are you planning to honor semver in the future? Is this a one off mistake? |
|
It's especially complex because some of my projects use this crate in their public API, so I need to bump semver even if you don't... |
The first three are all entangled because of a messy oci-spec semver incompatibility youki-dev/oci-spec-rs#288 Bumping composefs-rs is just to avoid having two versions of the proxy in the lockfile. Closes: bootc-dev#1567 Signed-off-by: Colin Walters <walters@verbum.org>
|
@cgwalters Thanks for your kind words.
It's just the maintainer (me) mistake. But I believe if we don't systematize it, we'll make the same mistakes over and over again. Do you have any good ideas? |
|
Personally I just have a mental checklist when reviewing PRs for "will we need a semver bump" and that's been mostly OK. But it does look like the semver checks GHA is fairly widely used https://github.com/search?q=-is%3Afork+path%3A.github+%22uses%3A+obi1kenobi%2Fcargo-semver-checks-action%40v2%22&type=code - including by hyper and rustls. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
According to the OCI Image Specification, the
historyfield is defined as optional:Which issue(s) this PR fixes:
Fixes #287
Special notes for your reviewer:
I added some basic unit tests, let me know if they're excessive/too verbose.
Does this PR introduce a user-facing change?