-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Diagnostic mem leak tutorial #14110
Diagnostic mem leak tutorial #14110
Conversation
f65d7ee
to
91fcc7d
Compare
@samsp-msft I just rebased this on the #13981 PR. It should represent what I have so far. Feedback would be appreciated. |
24ddf26
to
cb060ee
Compare
cb060ee
to
b60cb20
Compare
@mairaw I would like a first review when you get chance |
2a6ee44
to
6b81f02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make this less overwhelming, I've entered comments for the first article only and we can go from there.
Add tutorials to walk through key use cases.
Remove tutorial overview Use dotnetcli or console as appropriate Add applies to comment Add preparing sections
Fix URLs Fix commands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sdmaclea. Looks great. Left some comments for you to consider.
ms.author: stmaclea | ||
ms.date: 10/14/2019 | ||
--- | ||
# Sample debug target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it better to call this "Sample .NET Core diagnostics app" or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sample debug target
does sound a bit too formal. I don't like "Sample .NET Core diagnostics app" it doesn't convey the bad examples message. I did a quick look for common terminology, but didn't find anything. I chose to keep it as is.
@mairaw I applied most of your suggestions as is. Feel free to make any final edits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Add periods.
Prefer the readme in the samples repo
Add periods Consistent list marker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @sdmaclea! I've made some minor changes but looks good to publish.
Summary
Add tutorials to walk through key use cases