Skip to content

Port System.IO.Pipelines source code comments to Docs #2304

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Apr 26, 2019
Merged

Port System.IO.Pipelines source code comments to Docs #2304

merged 10 commits into from
Apr 26, 2019

Conversation

carlossanlop
Copy link
Contributor

Summary

Porting the source code comments I found in System.IO.Pipelines to Docs.

Fixes #Issue_Number (if available)

@carlossanlop
Copy link
Contributor Author

@mairaw please take a look whenever you get a chance.
Adding the source code owners @pakrym, @davidfowl in case you want to review the comments. I ported them with no modifications.

@rpetrusha rpetrusha added this to the April 2019 milestone Apr 16, 2019
rpetrusha
rpetrusha suggested changes Apr 16, 2019
View reviewed changes
Copy link

@rpetrusha rpetrusha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for documenting these, @carlossanlop. There were some additional members that didn't have triple-slash comments in the source; could you add the documentation for them as well? (My comments include the documentation to be added.)

Ron Petrusha and others added 5 commits April 16, 2019 12:27
@carlossanlop
Update xml/System.IO.Pipelines/PipeReader.xml
06a7f1d 
Thank you very much for adding the missing returns value.

Co-Authored-By: carlossanlop <1175054+carlossanlop@users.noreply.github.com>
@carlossanlop
Update xml/System.IO.Pipelines/PipeReader.xml
ebc6039 
Nice catch. Thank you for the correction.

Co-Authored-By: carlossanlop <1175054+carlossanlop@users.noreply.github.com>
@carlossanlop
Update xml/System.IO.Pipelines/StreamPipeExtensions.xml
a93d8cc 
Co-Authored-By: carlossanlop <1175054+carlossanlop@users.noreply.github.com>
@carlossanlop
Added missing comments suggested by rpetrusha in PR.
ecf2596
@carlossanlop
Merge branch 'pipelines' of https://github.com/carlossanlop/dotnet-ap…
4b6a5b2 
…i-docs into pipelines
@carlossanlop
Copy link
Contributor Author

The build error message says it "failed to load files". Is there anything I should do from my side?
@mairaw @rpetrusha

@mairaw
Copy link
Contributor

mairaw commented Apr 22, 2019

Nope @carlossanlop. A deployment happened today that it might be impacting this. I'll check.

@mairaw mairaw closed this Apr 22, 2019
@mairaw mairaw reopened this Apr 22, 2019
@mairaw mairaw closed this Apr 23, 2019
@mairaw mairaw reopened this Apr 23, 2019
@carlossanlop
Copy link
Contributor Author

Thank you @mairaw for catching that close tag. Not sure how it disappeared. I probably removed it by accident manually, I don't think it was the tool's fault.

rpetrusha
rpetrusha approved these changes Apr 25, 2019
View reviewed changes
Copy link

@rpetrusha rpetrusha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, @carlossanlop. I'll approve now and merge when the build completes successfully.

analogrelay
analogrelay approved these changes Apr 25, 2019
View reviewed changes
mairaw
mairaw reviewed Apr 26, 2019
View reviewed changes
Copy link
Contributor

@mairaw mairaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of suggestions since the build has some warnings

@carlossanlop
Apply suggestions from code review
9507a5a 
Suggestions from anurse and mairaw.

Co-Authored-By: carlossanlop <1175054+carlossanlop@users.noreply.github.com>
@rpetrusha rpetrusha merged commit da91ecf into dotnet:master Apr 26, 2019
@mairaw mairaw added new-content Indicates PRs that contain new articles and removed verify-build-before-merge labels Apr 26, 2019
@carlossanlop carlossanlop deleted the pipelines branch September 22, 2020 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mairaw mairaw mairaw left review comments

+4 more reviewers

@analogrelay analogrelay analogrelay approved these changes

@davidfowl davidfowl davidfowl approved these changes

@pakrym pakrym pakrym approved these changes

@rpetrusha rpetrusha rpetrusha approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@carlossanlop carlossanlop

Labels
new-content Indicates PRs that contain new articles
Projects
None yet
Milestone
April 2019
Development

Successfully merging this pull request may close these issues.
6 participants
@carlossanlop @mairaw @analogrelay @davidfowl @pakrym @rpetrusha