-
Notifications
You must be signed in to change notification settings - Fork 94
factor out codecs and core traits #325
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Some feedback on the code
@koehlma any plans to continue on this PR? |
Yeah, I still think it makes sense to separate those out. I would need to have a look at the changes that happened in the meantime, as they seem to break part of this PR. There was only one failing test (prior to the conflicting changes), however, I did not understand why it was failing. It would be great if there was someone who would look into that, if that were to happen again, so that we can see this through together. I do not quite understand the A-semver-major label: This PR is designed to not be a breaking change. At least this was the intention and I don't see where it would break semver compatibility. |
I think it might be due to new crate added. |
1 similar comment
I think it might be due to new crate added. |
Can you resolve the conflicts please, so that it would trigger another CI run? |
This PR fixes #324.
It introduces two new crates:
compression-core
: Contains theDecode
andEncode
traits.compression-codecs
: Contains the formercodec
module.I ran the changes through
cargo semver-checks
and it did not complain.Along the way, I made some decisions that may require further discussion:
async-compression
does not expose any of the data structures of the other crates. In particular, I did not move theLevel
enum and the moduleszstd
andbrotli
intocompression-codecs
. This enables future breaking changes incompression-codecs
without breakingasync-compression
.compression-codecs
re-exports the underlying compression libraries. The root crate does not directly depend on any of the libraries anymore. I migrated everything, including the tests, to the re-exported libraries. It may be worthwhile to add additional abstractions tocompression-codecs
to hide the underlying libraries. However, I think, that this should not be part of this PR, as it is already rather huge.