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

Port to ocidir 0.3, containers-image-proxy 0.7 #663

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

jeckersb
Copy link
Contributor

Signed-off-by: John Eckersberg [email protected]

Cargo.toml Outdated Show resolved Hide resolved
@cgwalters cgwalters changed the title Port to ocidir 0.3 Port to ocidir 0.3, containers-image-proxy 0.7 Sep 19, 2024
@cgwalters
Copy link
Member

Ah sorry, my porting to thiserror broke this one. I think we may just need a few .map_err(Into::into)?

@jeckersb
Copy link
Contributor Author

Ah sorry, my porting to thiserror broke this one. I think we may just need a few .map_err(Into::into)?

Holy cow is that an absolutely cryptic compiler error now 😆

Update containers-image-proxy to 0.7.0

Signed-off-by: John Eckersberg <[email protected]>
}
};

let driver = async { driver.await.map_err(Into::into) };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems to be enough to accomodate the thiserror change, took me longer than I would care to admit to understand the compiler error before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I struggled a lot with this when I was learning Rust for sure. The thing that is subtle is that the ? operator automatically also does Into::into, which means in cases where you're changing the error type (like this) but not using ? (which happens in most cases) you suddenly have to think about these conversions.

@cgwalters
Copy link
Member

Ah, test run hit #664

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but we also need to remember this is a semver-breaking change for us, so we'll need to bump the version for the next release.

@cgwalters
Copy link
Member

Fallout in containers/bootc#800

I have yanked version 0.15.0.

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.

2 participants