Skip to content

Drop some minimally-used dependencies#2364

Merged
rhatdan merged 2 commits intocontainers:mainfrom
mtrmac:drop-deps
Apr 6, 2024
Merged

Drop some minimally-used dependencies#2364
rhatdan merged 2 commits intocontainers:mainfrom
mtrmac:drop-deps

Conversation

@mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Apr 3, 2024

… to try and decrease our exposure to risk, where it is trivially cheap to do so.

See individual commit messages for details.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Apr 3, 2024

See also #2365.

@rhatdan
Copy link
Member

rhatdan commented Apr 4, 2024

LGTM

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

return err
}
if _, err := io.Copy(dest, src); err != nil {
_ = dest.Close()
Copy link
Member

Choose a reason for hiding this comment

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

Why not just defer the dest.Close()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn’t want just defer dest.Close() because a failure in Close after writing could have been significant.

But that’s a good point — doing the error checking in a defer is not actually all that more complex, and better for peace of mind. Fixed.

@mtrmac mtrmac added the kind/feature A request for, or a PR adding, new functionality label Apr 5, 2024
mtrmac added 2 commits April 5, 2024 18:48
This is mostly a formality, we still depend on it indirectly.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@rhatdan
Copy link
Member

rhatdan commented Apr 6, 2024

LGTM

@rhatdan rhatdan merged commit 07ef17d into containers:main Apr 6, 2024
@mtrmac mtrmac deleted the drop-deps branch April 8, 2024 12:20
@alexandear
Copy link

This was partially implemented in #2239.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature A request for, or a PR adding, new functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants