Skip to content

Equivalent usages section in TCP #32106

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

Merged
merged 20 commits into from
Nov 10, 2022

Conversation

liveans
Copy link
Member

@liveans liveans commented Nov 1, 2022

Summary

Adding an article to show TcpClient usages equivalent to Socket under Use TcpClient and TcpListener.

Fixes one task from #31702

Preview

https://review.learn.microsoft.com/en-us/dotnet/fundamentals/networking/sockets/tcp-classes?branch=pr-en-us-32106

@liveans
Copy link
Member Author

liveans commented Nov 1, 2022

Creating new article is harder than I expected, can you do a quick review on it @IEvangelist @antonfirsov?

Copy link
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

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

A few nits.

@antonfirsov
Copy link
Member

@IEvangelist are you aware about extensive docs on xref?

@IEvangelist
Copy link
Member

@IEvangelist are you aware about extensive docs on xref?

Hi @antonfirsov - I'm not sure I follow your question here, are you asking if we have docs on how to write an xref? If so we have internal content on this here: https://review.learn.microsoft.com/en-us/help/platform/links-how-to?branch=main#xref-cross-reference-links

@antonfirsov
Copy link
Member

@IEvangelist those docs are exactly what we need, thank you!

@IEvangelist
Copy link
Member

@IEvangelist those docs are exactly what we need, thank you!

Yeah, the best way to use them is to rely on the Learn Authoring Pack extension in VS Code.

@liveans
Copy link
Member Author

liveans commented Nov 4, 2022

@antonfirsov Should we add more sections? Thoughts?

@liveans liveans marked this pull request as ready for review November 4, 2022 18:29
@liveans liveans requested a review from a team as a code owner November 4, 2022 18:29
@liveans liveans requested a review from a team November 4, 2022 18:38
Copy link
Member

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

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

We should try to be less verbose. This addition is intended to be a tutorial and general explanation, we don't have to explain all the details IMO.

@liveans liveans self-assigned this Nov 7, 2022
Copy link
Member

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

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

We should figure out how to create / fix the API links before merging.

@gewarren
Copy link
Contributor

gewarren commented Nov 7, 2022

We should figure out how to create / fix the API links before merging.

This extension for VS Code makes it easy to insert xref (API) links: https://marketplace.visualstudio.com/items?itemName=docsmsft.docs-markdown

F1 -> Learn: Link to xref -> [type in some part of the API you want to insert]

liveans and others added 2 commits November 7, 2022 19:01
Co-authored-by: Anton Firszov <antonfir@gmail.com>
Copy link
Member

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

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

I have no more fundamental concerns about the structure/style.

@gewarren or @IEvangelist can you please take a look at the grammar/language here. There's a lot of text, and I'm not native.

liveans and others added 4 commits November 9, 2022 22:49
Copy link
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

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

I have several nits, a few grammatical suggestions, some code adjustments, and a question or two. Otherwise, it looks good. I was also going to ask, why are there no using statements? I would like to see anywhere that we're showing IDisposable implementations that we're either showing them in a using, does that make sense? For example, instead of:

var client = new TcpClient();
NetworkStream stream = client.GetStream();

We'd show:

using var client = new TcpClient();
using NetworkStream stream = client.GetStream();

@IEvangelist
Copy link
Member

I have no more fundamental concerns about the structure/style.

@gewarren or @IEvangelist can you please take a look at the grammar/language here. There's a lot of text, and I'm not native.

Hi @antonfirsov and @liveans - I have provided a review.

@liveans
Copy link
Member Author

liveans commented Nov 10, 2022

Hi @antonfirsov and @liveans - I have provided a review.

Thanks for the review @IEvangelist !

I would like to see anywhere that we're showing IDisposable implementations that we're either showing them in a using, does that make sense?

Well, it doesn't have a specific reason, tbh. I was focusing on equivalent code blocks, instead of those, but it makes sense to me. I'll address them with the next commit.

liveans and others added 2 commits November 10, 2022 15:20
Co-authored-by: David Pine <david.pine@microsoft.com>
@liveans
Copy link
Member Author

liveans commented Nov 10, 2022

Merging this, I can make another PR for any follow up comment. @antonfirsov @IEvangelist

@liveans liveans enabled auto-merge November 10, 2022 14:30
@liveans liveans disabled auto-merge November 10, 2022 15:17
@liveans liveans closed this Nov 10, 2022
@liveans liveans reopened this Nov 10, 2022
@liveans liveans enabled auto-merge (squash) November 10, 2022 15:47
@liveans liveans merged commit 0696283 into dotnet:main Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants