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
Document System.IO.Compression.Brotli.BrotliDecoder #3379
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.
LGTM aside from a couple of minor suggestions.
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.
Left some feedback. Follow other "OperationStatus-returning" API docs and add more details to the remarks for different cases and the guarantees the API makes for different returns (like overlapping buffers, same buffers).
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.
Thanks for your comments, @ahsonkhan and @tdykstra . I addressed them but have not commited them. Would you mind taking another look?
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.
Looks good, one additional suggestion.
Co-Authored-By: Tom Dykstra <tdykstra@microsoft.com>
@tdykstra I addressed the comments. If the build has no warnings and you agree with the applied suggestions, can you please get this merged? |
The return value can be as follows: | ||
- <xref:System.Buffers.OperationStatus.Done?displayProperty=nameWithType>: `source` was successfully and completely decompressed into `destination`. | ||
- <xref:System.Buffers.OperationStatus.DestinationTooSmall?displayProperty=nameWithType>: There is not enough space in `destination` to decompress `source`. | ||
- <xref:System.Buffers.OperationStatus.NeedMoreData?displayProperty=nameWithType>: The decompression action is partially done at least one more byte is required to complete the decompression task. This method should be called again with more input to decompress. |
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.
- <xref:System.Buffers.OperationStatus.NeedMoreData?displayProperty=nameWithType>: The decompression action is partially done at least one more byte is required to complete the decompression task. This method should be called again with more input to decompress. | |
- <xref:System.Buffers.OperationStatus.NeedMoreData?displayProperty=nameWithType>: The decompression action is partially done. At least one more byte is required to complete the decompression task. This method should be called again with more input to decompress. |
As described in the API proposal https://github.com/dotnet/corefx/issues/25785 and the PR that introduced this code dotnet/corefx#26229 .