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

check writer level when use NewWriterLevel #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

heartlock
Copy link

Signed-off-by: luke chenjinguang@huawei.com

Signed-off-by: luke <chenjinguang@huawei.com>
@heartlock
Copy link
Author

heartlock commented Mar 10, 2022

@andybalholm please have a look. I want to import this package to kubernetes. But there are still some small flaws. if you want to know more info about this thing, please see kubernetes/enhancements#3235

@andybalholm
Copy link
Owner

Adding an error return to NewWriterLevel is an API change. Doing that would require releasing a new major version of the package.

Instead, we should replace levels less than BestSpeed with BestSpeed, and replace levels higher than BestCompression with BestCompression. This should be done in NewWriterOptions rather than in NewWriterLevel, to take care of both entry points at once.

@heartlock
Copy link
Author

heartlock commented Mar 11, 2022

IMO, users should know this error just like gzip lib.

@andybalholm
Copy link
Owner

Yes, that's a fine API design. But it's not worth a breaking change.

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