-
Notifications
You must be signed in to change notification settings - Fork 6k
Review TODO code snippet references (docs) #1982
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
Conversation
The following comments apply to files that were edited in this PR: docs/framework/winforms/controls/how-to-access-the-managed-html-document-object-model.md docs/framework/wcf/how-to-specify-a-client-binding-in-configuration.md docs/framework/wcf/feature-details/create-an-ajax-wcf-asp-net-client.md docs/framework/wcf/how-to-secure-a-service-with-windows-credentials.md docs/framework/wcf/extending/how-to-inspect-and-modify-messages-on-the-service.md docs/framework/wcf/creating-ws-i-basic-profile-1-1-interoperable-services.md docs/framework/interop/platform-invoke-examples.md For all of these files, I added %20 to the directory name in all snippet links because the paths wouldn't resolve otherwise: |
@tompratt-AQ, note that there are publishing errors. |
@tompratt-AQ This looks good, aside from whatever build errors there may be. Assuming that you can fix them, I'll approve the PR. |
Thanks. I'll work on these build errors. I'm not too surprised by them, given that I'm trying to resolve previously broken snippet links. |
I agree with @rpetrusha. We can also start renaming some samples if it makes it easier to link to them (for example, to not have spaces in the file name). We just need to make sure to rename where they're referenced if you do that. |
It's not really the file name in this case, it's the folder name. For example, this path apparently builds but does not render the snippet on our doc page (“DataWorks SampleApp.OleDb” is the directory with the space): I noticed that the path would resolve in my VS Code preview when I added%20, like this: However, that causes a build error for “unable to resolve….cannot find reference” Any ideas? |
Resolved all of the build errors that I could, and put back the TODO tags on snippets that still have errors due to a bug. |
Added too many spaces to a few links, and missed some link breaks. |
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.
Looks great overall! Left some comments.
@@ -253,9 +254,9 @@ Class EntityClientSample | |||
End Class | |||
``` | |||
|
|||
In C#" | |||
In C#: |
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.
you don't need this anymore.
@@ -338,7 +339,7 @@ End Class | |||
|
|||
In C#: |
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.
ditto
@@ -24,7 +24,7 @@ The examples in this topic demonstrate how to use the <xref:System.Linq.Enumerab | |||
|
|||
The examples in this topic use the following `using`/`Imports` statements: | |||
|
|||
<!-- TODO: review snippet reference [!CODE [DP LINQ to DataSetExamples#ImportsUsing](DP LINQ to DataSetExamples#ImportsUsing)] --> | |||
[!code-csharp[DP LINQ to DataSetExamples#ImportsUsing](../../../../samples/snippets/csharp/VS_Snippets_ADO.NET/DP LINQ to DataSet Examples/CS/Program.cs#importsusing)] |
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.
There's also the VB version for these. Please add to all the LINQ to DataSet Examples references.
Thanks for the helpful review comments! |
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 @tompratt-AQ! Let's merge this.
Title
Review TODO code snippet references and resolve them.
Summary
This PR covers all of the 25 conceptual files (\docs) (48 instances) that are listed in the dotnet/docs Project "TODO: review snippet reference".
Suggested Reviewers
@mairaw