-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Validate CompressionLevel value is defined in BrotliStream ctor #57561
Validate CompressionLevel value is defined in BrotliStream ctor #57561
Conversation
Tagging subscribers to this area: @dotnet/area-system-io-compression Issue DetailsFix #46595
|
Added When you commit this breaking change:
Tagging @dotnet/compat for awareness of the breaking change. |
@stephentoub Can you please send the breaking change suggestion to the DL? dotnet/docs#25665 |
if it's merged |
The CI results are already deleted. I want to make sure we don't have unit test failures. Closing and reopening to re-trigger the CI (same with the other related PR). |
...ibraries/System.IO.Compression.Brotli/src/System/IO/Compression/enc/BrotliStream.Compress.cs
Outdated
Show resolved
Hide resolved
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.
Please see the comment from @scalablecory here: #57561 (comment)
f335271
to
5c98a3d
Compare
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.
I'm signing off, although I added the last commits to address the suggestion and to add tests. @jozkee @adamsitnik @jeffhandley @stephentoub would you mind giving us an additional sign-off if you agree with the change?
@carlossanlop were you planning to create a breaking change document for this? |
We should also consider updating this blog: https://devblogs.microsoft.com/dotnet/introducing-support-for-brotli-compression/ It explicitly calls out this behavior as a feature:
BrotliStream brotli = new BrotliStream(baseStream,
CompressionMode.Compress,
leaveOpen,
bufferSize,
(CompressionLevel)5); // Use level 5 I stumbled on it from searching for "brotli compression kestrel". cc @terrajobst |
I've removed it. |
breaking change doc created dotnet/docs#32620. |
Fix #46595