Update the IBufferWriter contract definition doc to be clearer #1949
Conversation
See dotnet/corefx#35554 for changes to the source. Please review if the formatting and style/tone is appropriate. cc @mairaw
|
closing and reopening to start a new build. |
| <summary>Returns a <see cref="T:System.Memory`1" /> to write to that is at least the requested size (specified by <paramref name="sizeHint" />).</summary> | ||
| <returns>A <see cref="T:System.Memory`1" /> of at least the size <paramref name="sizeHint" />. If <paramref name="sizeHint" /> is 0, returns a non-empty buffer.</returns> | ||
| <remarks>There is no guarantee that successive calls will return the same buffer or the same-sized buffer.</remarks> | ||
| <remarks>This must never return an empty <see cref="T:System.Memory`1" /> but it can throw <see cref="System.OutOfMemoryException"/> if the requested buffer size is not available.</remarks> |
There was a problem hiding this comment.
I don't think implementations should throw OutOfMemoryException neither I think we should prescribe exception type here at all.
There was a problem hiding this comment.
So remove the whole exception clause all together OR at least mention that it can throw?
Option 1:
<remarks>This must never return an empty <see cref="T:System.Memory`1" />.</remarks>
Option 2:
<remarks>This must never return an empty <see cref="T:System.Memory`1" /> but it can throw if the requested buffer size is not available.</remarks>
|
LGTM |
|
We need to fix the build errors before we merge this. @mairaw @rpetrusha do you have any idea what's wrong? I looked at the PR and nothing looked obvious to me. |
| <summary>Requests a span that is at least <paramref name="sizeHint" /> in size if possible.</summary> | ||
| <returns>A span of size <paramref name="sizeHint" />, if it's available; otherwise, a memory span with the maximum available memory. If <paramref name="sizeHint" /> is 0, returns currently available memory.</returns> | ||
| <remarks>To be added.</remarks> | ||
| <param name="sizeHint">The minimum length of the returned <see cref="T:System.Span`1" />. If 0, some, non-empty buffer is returned. </param> |
There was a problem hiding this comment.
I think this is the problem @BillWagner. The description for the param should have been updated on line 107 instead of added here. Same for the returns and remarks that are also duplicated here.
There was a problem hiding this comment.
Thanks, @mairaw. I'd looked over this file multiple times and completely missed the duplication each time!
BillWagner
left a comment
BillWagner
left a comment
There was a problem hiding this comment.
Thanks to @mairaw for the sharp 👀 and finding the build error.
This is ready to 
Update the docs based on feedback from dotnet#1949 / dotnet/corefx#35554
Update the docs based on feedback from #1949 / dotnet/corefx#35554
6 participants
See dotnet/corefx#35554 for changes to the source.
Please review if the formatting and style/tone is appropriate.
Also, how do we generally add multiple remarks?
cc @mairaw