Skip to content

Conversation

@thaJeztah
Copy link
Member

No description provided.

@thaJeztah thaJeztah force-pushed the move_compression branch 3 times, most recently from c77f9a7 to 101e3d1 Compare April 15, 2025 07:23
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
"Uncompressed" is 0, and the default value.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
type Compression int

const (
Uncompressed Compression = 0 // Uncompressed represents the uncompressed.
Copy link
Member Author

Choose a reason for hiding this comment

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

This could be None instead of Uncompressed as suggested in moby/moby#49167 (comment)

I kept it to align with containerd, but TBD I guess?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not really a blocker for me, but I would be much more in favor of None (compression.None instead of compression.Uncompressed) 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

OK; done!

I also swapped the order of commits, doing the test-table changes first (before the move)

type Compression int

const (
Uncompressed Compression = 0 // Uncompressed represents the uncompressed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really a blocker for me, but I would be much more in favor of None (compression.None instead of compression.Uncompressed) 😅

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
TODO: align with containerd pkg/archive/compression, which

- also moved the utilities to Compress/Decompress streams
- changed the Extension to not include ".tar"

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's only used in tests now, so move it there.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah requested a review from vvoland April 16, 2025 09:03
@thaJeztah
Copy link
Member Author

@vvoland PTAL - looks like branch protection on this repo requires "re-approval"

@thaJeztah thaJeztah merged commit c0a4ff3 into moby:main Apr 16, 2025
12 checks passed
@thaJeztah thaJeztah deleted the move_compression branch April 16, 2025 10:35
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