Review and fix TODO code snippet references (docs 2) #2023

tompratt-AQ · 2017-04-26T03:57:33Z

Title

Review and fix all TODO code snippet references. (\docs Part 2)

Summary

This PR covers the rest of the conceptual files (\docs) that are listed in the dotnet/docs Project "TODO: review snippet reference". It also fixes issues with .config files from the previous #1982 by calling them with an XML code tag.

Details

Where there are two identical config files, the one in the language folder (and common subfolder) is used instead of the common folder.

Suggested Reviewers

@mairaw

tompratt-AQ · 2017-04-26T04:05:36Z

The following comments are for the changes in this PR:

The file docs/framework/wcf/feature-details/how-to-host-a-wcf-service-in-iis.md has an inline xml snippet that is the same as the xml .config snippet file that is being called. Should the inline code be replaced?
I added code identifiers to several inline code snippets in some of the files.
I changed snippet tags from csharp style comments to XML style comments in some config files. Since we are now calling them with an XML code tag, I was worried it might not find them.

rpetrusha · 2017-04-26T16:50:51Z

@tompratt-AQ, to respond to your question in #1, if there's a choice between incline code and external code, it's best to use external code. (It's more maintainable.)

rpetrusha

LGTM, except for one comment about the final character in one C# source code file. And, in response to my last comment, replacing inline code with an external code reference.

rpetrusha · 2017-04-26T17:10:11Z

samples/snippets/csharp/VS_Snippets_CFX/c_standalonedatacontractserializer/cs/source.cs

@@ -42,19 +42,19 @@ public class LibraryPatron
        [DataContract]
        public class LibraryItem
        {
-            //code not shown�
+            //code not shown�


What is the final character (rendered as a question mark in a triangle) supposed to be? Make sure the file is saved using UTF-8 encoding (it's probably saved using ASCII encoding).

tompratt-AQ · 2017-04-26T20:25:11Z

I addressed Ron's comments, fixed 4 more files, and fixed one file by re-adding snippets.

Once these clear, there is only one file left in \docs with "<!-- TODO: review snippet reference" in it:
docs/standard/base-types/how-to-display-localized-date-and-time-information-to-web-users.md which has a snippet tag that links to an .aspx file. Before filing a bug on the issue, it seems like there should be a way to make it work with our current snippet tag system.

tompratt-AQ · 2017-04-26T22:34:33Z

Filed issue 982841 about the code snippet that links to the .aspx files. So, we'll leave that TODO as the last one in docs. If everything else checks out here, this should be ready to go!

mairaw

This is great @tompratt-AQ! Left a few comments.

mairaw · 2017-04-27T05:49:18Z

docs/framework/wcf/creating-ws-i-basic-profile-1-1-interoperable-services.md

- [!code-vb[C_HowTo-WCFServiceAndASMXClient#0](../../../samples/snippets/visualbasic/VS_Snippets_CFX/c_howto-wcfserviceandasmxclient/vb/program.vb#0)]  
- [!code-xml[C_HowTo-WCFServiceAndASMXClient#1](../../../samples/snippets/common/VS_Snippets_CFX/c_howto-wcfserviceandasmxclient/common/app.config#1)]
+ [!code-vb[C_HowTo-WCFServiceAndASMXClient#0](../../../samples/snippets/visualbasic/VS_Snippets_CFX/c_howto-wcfserviceandasmxclient/vb/program.vb#0)] 
+ [!code-xml[C_HowTo-WCFServiceAndASMXClient#1](../../../samples/snippets/csharp/VS_Snippets_CFX/c_howto-wcfserviceandasmxclient/common/app.config#1)]        


I'd say as we start changing the references from common to csharp that we also pull the snippet out at the same time and also make sure no other topic is referencing that file.

mairaw · 2017-04-27T05:52:40Z

docs/framework/wcf/feature-details/config-wcf-service-with-aspnet-web-service.md

@@ -47,7 +47,8 @@ To configure a [!INCLUDE[indigo1](../../../../includes/indigo1-md.md)] service e

 [!code-csharp[C_HowTo-WCFServiceAndASMXClient#0](../../../../samples/snippets/csharp/VS_Snippets_CFX/c_howto-wcfserviceandasmxclient/cs/program.cs#0)] 
 [!code-vb[C_HowTo-WCFServiceAndASMXClient#0](../../../../samples/snippets/visualbasic/VS_Snippets_CFX/c_howto-wcfserviceandasmxclient/vb/program.vb#0)] 
- <!-- TODO: review snippet reference [!code[C_HowTo-WCFServiceAndASMXClient#1](../../../../samples/snippets/common/VS_Snippets_CFX/c_howto-wcfserviceandasmxclient/common/app.config#1)] -->    
+ [!code-xml[C_HowTo-WCFServiceAndASMXClient#1](../../../../samples/snippets/csharp/VS_Snippets_CFX/c_howto-wcfserviceandasmxclient/common/app.config#1)] 
+ [!code-xml[C_HowTo-WCFServiceAndASMXClient#1](../../../../samples/snippets/visualbasic/VS_Snippets_CFX/c_howto-wcfserviceandasmxclient/common/app.config#1)]   


remove this duplicate snippet.

mairaw · 2017-04-27T05:58:35Z

samples/snippets/common/VS_Snippets_CFX/c_howto_configureclientbinding/common/client.exe.config

-//<snippet100>
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="utf-8" ?>   
+<!-- <snippet100> -->


shouldn't this come before as in the original?

mairaw · 2017-04-27T05:58:44Z

samples/snippets/common/VS_Snippets_CFX/c_howto_createreliablesessionhttps/common/app.config


 <?xml version="1.0" encoding="utf-8" ?>
+<!-- <snippet2211> -->


same here and in some of the samples below

tompratt-AQ · 2017-04-28T00:00:06Z

@mairaw In this last commit, I did the following:

Fixed the specific issues that you flagged in your last comments.
For all config files that were in TODO links in this PR: I confirmed that there were duplicates between the Common branch and the csharp branch, made sure that any other reference to the Common file were changed, and then deleted the config file in the Common branch.
Deleted other config files that were in the same folder as the deleted dupes after confirming that they also were dupes and weren't being referenced.

mairaw

It's great to get these fixes in. Thanks @tompratt-AQ!

Review and fix TODO code snippet references (docs 2)

bc70902

dnfclas added the cla-already-signed label Apr 26, 2017

Fix build error

51fa5cf

rpetrusha approved these changes Apr 26, 2017

View reviewed changes

Add snippet files and address review comments

0f05799

Merge branch 'master' into Review-Snippet-References-Docs-2

82e8f0e

mairaw requested changes Apr 27, 2017

View reviewed changes

Make changes per review comments and delete duplicate .config files

d63b36c

mairaw closed this May 1, 2017

mairaw reopened this May 1, 2017

dnfclas added the cla-already-signed label May 1, 2017

mairaw approved these changes May 2, 2017

View reviewed changes

mairaw merged commit 0a2acbf into dotnet:master May 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Review and fix TODO code snippet references (docs 2) #2023

Review and fix TODO code snippet references (docs 2) #2023

Uh oh!

tompratt-AQ commented Apr 26, 2017

Uh oh!

tompratt-AQ commented Apr 26, 2017

Uh oh!

rpetrusha commented Apr 26, 2017

Uh oh!

rpetrusha left a comment

Uh oh!

rpetrusha Apr 26, 2017

Uh oh!

tompratt-AQ commented Apr 26, 2017

Uh oh!

tompratt-AQ commented Apr 26, 2017

Uh oh!

mairaw left a comment

Uh oh!

mairaw Apr 27, 2017

Uh oh!

mairaw Apr 27, 2017

Uh oh!

mairaw Apr 27, 2017

Uh oh!

mairaw Apr 27, 2017

Uh oh!

tompratt-AQ commented Apr 28, 2017

Uh oh!

mairaw left a comment

Uh oh!

Uh oh!


		<?xml version="1.0" encoding="utf-8" ?>
		<!-- <snippet2211> -->

Review and fix TODO code snippet references (docs 2) #2023

Review and fix TODO code snippet references (docs 2) #2023

Uh oh!

Conversation

tompratt-AQ commented Apr 26, 2017

Title

Summary

Details

Suggested Reviewers

Uh oh!

tompratt-AQ commented Apr 26, 2017

Uh oh!

rpetrusha commented Apr 26, 2017

Uh oh!

rpetrusha left a comment

Choose a reason for hiding this comment

Uh oh!

rpetrusha Apr 26, 2017

Choose a reason for hiding this comment

Uh oh!

tompratt-AQ commented Apr 26, 2017

Uh oh!

tompratt-AQ commented Apr 26, 2017

Uh oh!

mairaw left a comment

Choose a reason for hiding this comment

Uh oh!

mairaw Apr 27, 2017

Choose a reason for hiding this comment

Uh oh!

mairaw Apr 27, 2017

Choose a reason for hiding this comment

Uh oh!

mairaw Apr 27, 2017

Choose a reason for hiding this comment

Uh oh!

mairaw Apr 27, 2017

Choose a reason for hiding this comment

Uh oh!

tompratt-AQ commented Apr 28, 2017

Uh oh!

mairaw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!