review github files #4

BillWagner · 2018-04-01T00:50:52Z

Reviewed the following files:

Readme.md was fine
Contributing.md was created and points to the docs repo.
Code_of_conduct.md was created and follows the docs repo.
.gitignore Created for samples and projects.
.github folder: created pull request template and codeowners file based on docs repository.

Reviewed the following files: Readme.md was fine Contributing.md was created and points to the docs repo. Code_of_conduct.md was created and follows the docs repo. .gitignore Created for samples and projects. .github folder: created pull request template and codeowners file based on docs repository.

pkulikov · 2018-04-01T06:19:23Z

@BillWagner Readme.md file also can be improved:

.NET Core documentation -> .NET documentation at the first sentence.
The last sentence of the first paragraph also should be rephrased to mention .NET documentation. This sentence looks to be a good candidate to include the link to the docs repo.
Add the link to Contributing.md to navigate first-comers (??)

mairaw

Current changes look good but readme.md file should also be changed. That file was written when we didn't have the snippets folder with samples that are specific to .NET Framework or written using the old SDK style. So it should probably be reviewed to be more generic and include some guidance for snippets. It also mentions .NET Core.

mairaw · 2018-04-02T16:29:02Z

.github/CODEOWNERS

+# C++ snippets
+/snippets/cpp/** @rpetrusha
+# C# Snippets:
+/snippets/csharp/** @BillWagner


this will be a lot of samples for you and Ron to review if you set the entire snippets/language to one of you

This is true, but that's what we had in the docs repo.

It has not been a lot of change so far.

@rpetrusha What do you think?

Sorry, @BillWagner. I didn't get email notification with your question. But I think that this is fine; there haven't been many changes.

If you guys are ok with it, I'm good

BillWagner · 2018-04-02T18:51:30Z

@mairaw I made a first stab at updating the readme. Let me know what you think should be added.

Snippets are smaller samples, but otherwise should be the same.

rpetrusha

This looks good, @BillWagner. I did leave a couple of comments, but you can merge when you decide what you'd like to address.

rpetrusha · 2018-04-04T17:06:26Z

.github/CODEOWNERS

+# C++ snippets
+/snippets/cpp/** @rpetrusha
+# C# Snippets:
+/snippets/csharp/** @BillWagner


Sorry, @BillWagner. I didn't get email notification with your question. But I think that this is fine; there haven't been many changes.

rpetrusha · 2018-04-04T17:07:56Z

README.md


 Some of the articles will have more than one sample associated with them. 

 The readme.md file for each sample will refer to the article so that
 you can read more about the concepts covered in each sample.

+There are two classes of samples in this repository:


perhaps "two classes of code" rather than two classes of samples, since one of the classes is samples

rpetrusha · 2018-04-04T17:10:46Z

README.md

@@ -31,15 +42,14 @@ Except where noted, all samples build from the command line on
 any platform supported by .NET Core. There are a few samples that are
 specific to Visual Studio and require Visual Studio 2017 or later. In 
 addition, some samples show platform specific features and will require 
-a specific platform.
+a specific platform. Other samples and snippets require the .NET Framework
+and will run on Windows platforms, and will need Visual Studio to build.


VS isn't required; the Developer Pack for a particular .NET Framework version is. (You can then compile with the command line.)

rpetrusha · 2018-04-04T17:11:45Z

.github/CODEOWNERS

+# Order is important. The last matching pattern has the most precedence.
+# The folders are ordered as follows:
+
+# In each subsection folders are order first by depth, then alphabetically.


nit: order --> ordered

mairaw

Thanks @BillWagner. This looks good but readme still needs some work I think.

mairaw · 2018-04-04T17:17:43Z

.github/CODEOWNERS

+# C++ snippets
+/snippets/cpp/** @rpetrusha
+# C# Snippets:
+/snippets/csharp/** @BillWagner


If you guys are ok with it, I'm good

mairaw · 2018-04-04T17:24:23Z

README.md

@@ -1,18 +1,29 @@
 # .NET Core Samples


remove Core from here

mairaw · 2018-04-04T17:24:33Z

README.md

@@ -1,18 +1,29 @@
 # .NET Core Samples

 This folder contains all the sample code that is part of any topic under


This folder -> This repo

mairaw · 2018-04-04T17:27:15Z

README.md

+- **Samples** represent programs that demonstrate apoplication or library scenarios. These samples are typically larger, and often use more than one technology, feature, or toolkit.
+- **Snippets** represent small focused examples that demonstrate one feature or syntax. These should be no more than a single screen of code.
+
+In both cases, we verify the samples in our CI system.


This is not true yet. Just samples are verified and we need @dend to turn them on here on the new repo.

Snippets should be verified as well. But because most of them don't contain a csproj file, we need to find ways to test them.

I changed it to reflect the current reality (but still encourage buildable projects moving forward.)

mairaw · 2018-04-04T17:35:10Z

README.md

 ## Building a sample

-You build the samples using the .NET Core CLI. You can download the CLI from
+You build any .NET core sample using the .NET Core CLI. You can download the CLI from


You usually download the SDK that comes with the CLI.

mairaw · 2018-04-04T17:35:48Z

README.md

 ## Building a sample

-You build the samples using the .NET Core CLI. You can download the CLI from
+You build any .NET core sample using the .NET Core CLI. You can download the CLI from
 [the .NET Core home page](http://microsoft.com/net/core). Then, execute


this url now is a get started page. not the best link now I'd say.

updated to https://www.microsoft.com/net/download

mairaw · 2018-04-04T17:37:40Z

README.md

+
+## Snippets
+
+Snippets are extracted from small programs that include the snippet. Snippets are all located in the top level **/snippets** folder. Snippets should follow all other sample guidance.


The only confusing part about this is that we have snippets that do require VS like WPF snippets. So if they follow the sample guidance where we only mention .NET Core projects. they might get confused.

I tried to rework this section. It's probably worth another review.

BillWagner · 2018-04-04T20:19:58Z

@mairaw I updated this based on your feedback.

pkulikov · 2018-04-04T20:32:22Z

README.md


 Some of the articles will have more than one sample associated with them. 

 The readme.md file for each sample will refer to the article so that
 you can read more about the concepts covered in each sample.

+There are two classes of code in this repository:
+
+- **Samples** represent programs that demonstrate apoplication or library scenarios. These samples are typically larger, and often use more than one technology, feature, or toolkit.


Typo: apoplication

These samples are typically larger... Not clear larger than what; obviously larger than snippets. Then snippets introduction should go before the samples introduction. That also would sync with the further content where snippets section goes before 'Building a sample' section

pkulikov · 2018-04-04T20:33:42Z

README.md

+
+## Snippets
+
+Snippets are extracted from small programs that include the snippet. Snippets are all located in the top level **/snippets** folder. While snippets are small blcoks of code, we want to move toward snippets that are part of buildable sample projects.


Typo in the last sentence: small blcoks

pkulikov · 2018-04-04T20:37:35Z

README.md

-You build the samples using the .NET Core CLI. You can download the CLI from
-[the .NET Core home page](http://microsoft.com/net/core). Then, execute
+You build any .NET core sample using the .NET Core CLI. The .NET Core CLI is installed with
+[the .NET Core SDK](https://www.microsoft.com/net/download). Then, execute
 these commands from the CLI in the directory of any sample:


In this paragraph three sentences: active - passive - active voice and a lot of .NET Core. Should we combine the first two sentences:
You build any .NET Core sample using the .NET Core CLI, which can be installed with [the .NET Core SDK]....
Also typo in the first sentence: .NET core

wow. Super quick feedback. 🏎

mairaw

I like the changes. Thanks @BillWagner!

mairaw · 2018-04-04T22:27:05Z

README.md

 are organized in sub-folders. These sub-folders are organized similar
-to the organization of the docs for .NET Core.
+to the organization of the docs for .NET.

 Some of the articles will have more than one sample associated with them. 

 The readme.md file for each sample will refer to the article so that


nit: should this be moved to line 16 where you specifically talk about samples? snippets won't have readme file.

Readme.md files are only included in samples.

BillWagner requested a review from mairaw April 1, 2018 00:50

mairaw reviewed Apr 2, 2018

View reviewed changes

update readme regarding snippets

424e388

Snippets are smaller samples, but otherwise should be the same.

rpetrusha approved these changes Apr 4, 2018

View reviewed changes

mairaw requested changes Apr 4, 2018

View reviewed changes

BillWagner added 2 commits April 4, 2018 16:00

respond to feedback.

c5fd41b

feedback II

fb0d083

pkulikov reviewed Apr 4, 2018

View reviewed changes

2nd round feedback

173e42e

mairaw approved these changes Apr 4, 2018

View reviewed changes

one quick final edit

a615a32

Readme.md files are only included in samples.

BillWagner merged commit 64c8152 into dotnet:master Apr 4, 2018

mairaw assigned BillWagner Apr 5, 2018

BillWagner deleted the update-infrastructure branch April 9, 2018 15:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

review github files #4

review github files #4

BillWagner commented Apr 1, 2018 •

edited

Loading

pkulikov commented Apr 1, 2018

mairaw left a comment

mairaw Apr 2, 2018

BillWagner Apr 2, 2018

rpetrusha Apr 4, 2018

mairaw Apr 4, 2018

BillWagner commented Apr 2, 2018

rpetrusha left a comment

rpetrusha Apr 4, 2018

rpetrusha Apr 4, 2018

rpetrusha Apr 4, 2018

rpetrusha Apr 4, 2018

mairaw left a comment

mairaw Apr 4, 2018

mairaw Apr 4, 2018

mairaw Apr 4, 2018

mairaw Apr 4, 2018

BillWagner Apr 4, 2018

mairaw Apr 4, 2018

mairaw Apr 4, 2018

BillWagner Apr 4, 2018

mairaw Apr 4, 2018

BillWagner Apr 4, 2018

BillWagner commented Apr 4, 2018

pkulikov Apr 4, 2018

pkulikov Apr 4, 2018

pkulikov Apr 4, 2018

BillWagner Apr 4, 2018

mairaw left a comment

mairaw Apr 4, 2018

		@@ -1,18 +1,29 @@
		# .NET Core Samples

		This folder contains all the sample code that is part of any topic under


		## Snippets

		Snippets are extracted from small programs that include the snippet. Snippets are all located in the top level /snippets folder. Snippets should follow all other sample guidance.

review github files #4

review github files #4

Conversation

BillWagner commented Apr 1, 2018 • edited Loading

pkulikov commented Apr 1, 2018

mairaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BillWagner commented Apr 2, 2018

rpetrusha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mairaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BillWagner commented Apr 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mairaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BillWagner commented Apr 1, 2018 •

edited

Loading