Skip to content
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

Create diagnosticsource-getting-started.md #29255

Merged
merged 29 commits into from
Jun 2, 2022

Conversation

mikelle-rogers
Copy link
Member

@mikelle-rogers mikelle-rogers commented May 2, 2022

Fixes #49082 and fixes #3029

This is a rough draft, all suggestions appreciated.
@mikelle-rogers mikelle-rogers requested a review from noahfalk May 5, 2022 23:54
@mikelle-rogers mikelle-rogers marked this pull request as ready for review May 9, 2022 17:09
@mikelle-rogers mikelle-rogers requested review from tommcdon and a team as code owners May 9, 2022 17:09
@mikelle-rogers mikelle-rogers requested a review from noahfalk May 9, 2022 17:09
Copy link
Member

@tommcdon tommcdon left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

Some general guidance that explains a lot of my comments:

  • Okay to use "you" but not "we" or "our".
  • Shorten sentences to help with localization.
  • Use active voice over passive.

Use you instead of we, shorten sentences to help with localization, use active voice, comments should be capitalized and have punctuation.
Copy link
Contributor

@josalem josalem left a comment

Choose a reason for hiding this comment

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

In general, I think we should consolidate the code samples into all-inclusive samples and then break down their contents. Users will want to be able to visit this guide and leave with a snippet of code they can build on. Having everything broken up makes that a complex task of grokking the whole page.

@gewarren
Copy link
Contributor

gewarren commented May 10, 2022

In general, I think we should consolidate the code samples into all-inclusive samples and then break down their contents. Users will want to be able to visit this guide and leave with a snippet of code they can build on. Having everything broken up makes that a complex task of grokking the whole page.

The best way to accomplish that would be to put all the code into a *.cs file and check that in with a *.csproj file so we can verify that it compiles. And then add snippet tags (you can name them anything; the format is // <yoursnippettag> to start the snippet and // </yoursnippettag> to end it) around the different sections that you want to break out in the tutorial.

The docs markdown to reference a snippet looks like:

:::code language="csharp" source="snippets/character-encoding-introduction/csharp/PrintStringChars.cs" id="SnippetPrintChars":::

And you could have a note in the tutorial with a link to the *.cs file to see the entire example? Or, if you think it's useful as a code sample, you can check it into the dotnet/samples repo and then link to the sample.

Add clarifying comments, URLs for the APIs and responded to suggestions.
Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

Can you change all the ```C# lines to ```csharp?
And for all the output code, they should be labeled as console.

Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

Please also add the file to the table of contents (toc.yml). Sorry I didn't catch that earlier.

@mikelle-rogers
Copy link
Member Author

mikelle-rogers commented May 24, 2022

@gewarren

Please also add the file to the table of contents (toc.yml). Sorry I didn't catch that earlier.

Only this file, or the entire diagnostics repo? Is this the toc.yml file you mentioned? https://github.com/dotnet/docs/blob/dev/mirogers/diagnosticsource-getting-started/docs/toc.yml

@gewarren
Copy link
Contributor

@gewarren

Please also add the file to the table of contents (toc.yml). Sorry I didn't catch that earlier.

Only this file, or the entire diagnostics repo? Is this the toc.yml file you mentioned? https://github.com/dotnet/docs/blob/dev/mirogers/diagnosticsource-getting-started/docs/toc.yml

Just this file, since it's new. The TOC file is at https://github.com/dotnet/docs/blob/main/docs/fundamentals/toc.yml.

@mikelle-rogers mikelle-rogers requested a review from gewarren May 25, 2022 09:40
docs/core/diagnostics/DiagnosticSource.csproj Outdated Show resolved Hide resolved
docs/fundamentals/toc.yml Outdated Show resolved Hide resolved
@mikelle-rogers mikelle-rogers merged commit 8d732e4 into main Jun 2, 2022
@mikelle-rogers mikelle-rogers deleted the dev/mirogers/diagnosticsource-getting-started branch June 2, 2022 17:21
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.

DiagnosticSource and DiagnosticListener Code example in DiagnosticSourceUsersGuide.md is outdated
7 participants