coreclr: Automatic port of triple slash from System.Buffers.Binary#2693
coreclr: Automatic port of triple slash from System.Buffers.Binary#2693mairaw merged 12 commits intodotnet:masterfrom carlossanlop:clr_buffers.binary
Conversation
mairaw
left a comment
There was a problem hiding this comment.
Thanks @carlossanlop. Left a few comments to be considered.
Co-Authored-By: Maira Wenzel <mairaw@microsoft.com>
…dotnet-api-docs into clr_buffers.binary
…rify no-op remarks.
|
@mairaw @rpetrusha I resolved all the suggestions, added extra params and returns, clarified the remarks. Can you please take a look? |
|
@layomia since you're the owner, can you also please take a quick look? |
|
@mairaw @rpetrusha the build passed. Is this good to merge? |
|
Please give me some time to review this before merging. Tyvm. |
Sure, @ahsonkhan. Please ping me and @rpetrusha when you're done reviewing this PR. |
mairaw
left a comment
There was a problem hiding this comment.
Made a few more changes. Please take a look.
|
Your changes look good @mairaw , thanks for making them. |
|
@ahsonkhan can we assume this looks good to merge? |
ahsonkhan
left a comment
There was a problem hiding this comment.
Other than adding exception tags to document Argument OOR exceptions, the rest of the feedback are nits/questions.
Looks good otherwise.
|
@mairaw I addressed all of Ahson's comments. He left you a couple of questions (a rewording which I decided to revert), so let me know if you'd like us to use your words instead. Does this look good to merge, if the build finishes successfully? |
ahsonkhan
left a comment
There was a problem hiding this comment.
Some leftover word-smithing suggestions.
|
I addressed all the feedback, @ahsonkhan @mairaw. |
|
@mairaw the build completed successfully and without warnings. Can we get it merged if the last changes look good to you? |
ahsonkhan
left a comment
There was a problem hiding this comment.
Use an for Int16/Int32/Int64, but a UInt16/UInt32/UInt64 (consistency with existing a/an usage).
Co-Authored-By: Ahson Khan <ahkha@microsoft.com>
|
@mairaw build was successful and with no warnings. The last comments were addressed. Is this good to merge? |
Found a bug in the tool that was preventing it from porting coreclr triple slash comments.
Area owners: @layomia, @JeremyKuhne, @ahsonkhan
Language owners: @mairaw @rpetrusha