project: Use ImageReference from containers_image_proxy#2204
project: Use ImageReference from containers_image_proxy#2204Johan-Liebert1 wants to merge 1 commit into
Conversation
Refactor `struct ImageReference` as an alias to `containers_image_proxy::ImageReference`, also refactor `enum Transport` as Transport from containers_image_proxy Rename `crate::lib::spec::ImageReference` to `ImageReferenceSig` to avoid confusion with ImageReference from containers_image_proxy Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
github-actions
Bot
added
area/install
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request renames the ImageReference struct to ImageReferenceSig across the library and replaces internal definitions of Transport and ImageReference in crates/ostree-ext with type aliases from containers_image_proxy. The review feedback identifies several opportunities to simplify the code by removing redundant Ok(...?) wrapping and explicit anyhow:: prefixes, as well as cleaning up a trailing doc comment in the container module.
| /// Parse a base [`ImageReference`] from a CLI argument. | ||
| pub fn parse_base_imgref(s: &str) -> Result<ImageReference> { | ||
| ImageReference::try_from(s) | ||
| Ok(ImageReference::try_from(s)?) |
There was a problem hiding this comment.
The use of Ok(...?) is redundant here as ImageReference::try_from(s) already returns a Result compatible with the function's return type.
| Ok(ImageReference::try_from(s)?) | |
| ImageReference::try_from(s) |
| let all_images = all_images | ||
| .into_iter() | ||
| .map(|img| crate::container::ImageReference::try_from(img.as_str())) | ||
| .map(|img| -> Result<_> { Ok(crate::container::ImageReference::try_from(img.as_str())?) }) |
There was a problem hiding this comment.
The closure can be simplified by removing the redundant Ok(...?) wrapping and explicit return type annotation, as try_from already returns the expected Result type.
| .map(|img| -> Result<_> { Ok(crate::container::ImageReference::try_from(img.as_str())?) }) | |
| .map(|img| crate::container::ImageReference::try_from(img.as_str())) |
| /// Combination of a remote image reference and transport. | ||
| /// | ||
| /// For example, |
There was a problem hiding this comment.
The doc comment "For example," is now trailing and refers to nothing since the struct definition and its fields were removed.
| /// Combination of a remote image reference and transport. | |
| /// | |
| /// For example, | |
| /// Combination of a remote image reference and transport. |
| }) | ||
| .chain(deployment_commits.into_iter().map(Ok)) | ||
| .collect::<Result<Vec<_>>>()?; | ||
| .collect::<anyhow::Result<Vec<_>>>()?; |
There was a problem hiding this comment.
The explicit anyhow:: prefix is redundant here as Result is already aliased to anyhow::Result in this module.
| .collect::<anyhow::Result<Vec<_>>>()?; | |
| .collect::<Result<Vec<_>>>()?; |
|
rerun |
2 participants
Refactor
struct ImageReferenceas an alias tocontainers_image_proxy::ImageReference, also refactorenum Transportas Transport from containers_image_proxyRename
crate::lib::spec::ImageReferencetoImageReferenceSigto avoid confusion with ImageReference from containers_image_proxyRequires: bootc-dev/containers-image-proxy-rs#138