Update .NET Core porting document to VS2017 #1768

conniey · 2017-03-21T21:13:57Z

Update .NET Core porting document to VS2017

Summary

Fixes #1680

Details

Removes content related to project.json and xproj
Removes suggestion about using PCLs since csproj supports all complex msbuild logic.
Updates examples to VS2017

Suggested Reviewers

@mairaw

mairaw · 2017-03-22T22:40:04Z

@guardrex can you help review this one?

guardrex · 2017-03-23T05:02:29Z

docs/core/porting/project-structure.md

@@ -4,7 +4,7 @@ description: Organizing Your Project to Support .NET Framework and .NET Core
 keywords: .NET, .NET Core


Put the title into sentence case:

Organizing your project to support .NET Framework and .NET Core

Use a real description line, such as:

Help for project owners who want to compile their solution against .NET Framework and .NET Core side-by-side.

... ☝️ lifted from the doc. It doesn't need to be that, of course. That's just a suggestion.

Add keywords (not necessarily these ... just suggestions):

, .NET Framework, project layout, multiple frameworks

guardrex · 2017-03-23T05:02:58Z

docs/core/porting/project-structure.md

@@ -13,38 +13,24 @@ ms.assetid: 3af62252-1dfa-4336-8d2f-5cfdb57d7724

 # Organizing Your Project to Support .NET Framework and .NET Core


Match casing of title

guardrex · 2017-03-23T05:14:35Z

docs/core/porting/project-structure.md

@@ -13,38 +13,24 @@ ms.assetid: 3af62252-1dfa-4336-8d2f-5cfdb57d7724

 # Organizing Your Project to Support .NET Framework and .NET Core

-> [!WARNING]
-> This topic hasn't been updated to the latest version of the tooling yet.
-
 This article is to help project owners who want to compile their solution against .NET Framework and .NET Core side-by-side.  It provides several options to organize projects to help developers achieve this goal.  Here are some typical scenarios to consider when you are deciding how to setup your project layout with .NET Core.  They may not cover everything you want; prioritize based on your project's needs.


This article helps project owners ...

They use the word "topic" quite a bit around here, but "article" isn't forbidden by the style manual.

Remove one space after the period ...

... this goal. Here are ...

I wonder about "Here" here. 😀 Would it be better as ...

The following list provides some typical scenarios ...

Use contraction ...

... when you're deciding ...

Remove a space after the period ...

... .NET Core. They may ...

"The list" over "They" ? ...

The list may not cover ...

guardrex · 2017-03-23T05:16:00Z

docs/core/porting/project-structure.md

-* [**Combine existing projects and .NET Core projects into single projects**][option-xproj]
-  
+* [**Combine existing projects and .NET Core projects into single projects**][option-csproj]
+
  *What this is good for:*
  * Simplifying your build process by compiling a single project rather than compiling multiple projects, each targeting a different .NET Framework version or platform.
  * Simplifying source file management for multi-targeted projects because you have to manage a single project file.  When adding/removing source files, the alternatives require you to manually sync these with your other projects.


I'm a fan of "must" over the state "have to" ...

... because you must manage a single project ...

Remove one space after the period ...

... project file. When ...

guardrex · 2017-03-23T05:23:28Z

docs/core/porting/project-structure.md

-  * Continuing to support development on existing projects without having to upgrade for developers/contributors who may not have Visual Studio 2015.
-  * Decreasing the possibility in creating new bugs in existing projects because no code churn is required in those projects.
+  *Unsupported scenarios:*
+  * Does not allow developers without Visual Studio 2017 to open existing projects. To support older versions of Visual Studio, [keeping your project files in different folders](#support-vs) is a better option.


Use contraction ...

Doesn't allow developers ...

"Doesn't" ... "without" ... double-negative-ish. How about? ...

Requires developers to use Visual Studio 2017 to open existing projects.

guardrex · 2017-03-23T06:21:29Z

samples/framework/libraries/migrate-library-csproj/src/Car/Car.csproj

+  <PropertyGroup>
+    <TargetFrameworks>net45;net451;netstandard1.6</TargetFrameworks>
+    <AssemblyName>Car</AssemblyName>
+    <PackageId>Car</PackageId>


I suggest dropping the AssemblyName and PackageId.

guardrex · 2017-03-23T06:22:23Z

samples/framework/libraries/migrate-library-csproj/src/Car/Car.csproj

+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFrameworks>net45;net451;netstandard1.6</TargetFrameworks>


Seems unusual to target net45 and net451. They're supported TFM's, but they're unsupported frameworks. Can it be net452;netstandard1.6? Also, I've noted that the dev repos have all gone net46. I'm not sure what the standard is for samples tho.

I wanted to show that the existing solution, which targeted net45 could still be built. I can do net45;net46;netstandard1.6?

How about net452 (still supported), net46, netstandard1.6? ... net45 falls out of the security update window.

guardrex · 2017-03-23T06:25:00Z

...es/framework/libraries/migrate-library-csproj-keep-existing/tests/Car.Tests/Car.Tests.csproj

+
+  <ItemGroup>
+    <Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
+  </ItemGroup>


Remove this ItemGroup ... it's an artifact. microsoft/vstest#472

guardrex · 2017-03-23T06:34:48Z

samples/framework/libraries/migrate-library-csproj/src/Car/Car.csproj

+    <Reference Include="System.Collections.Concurrent" />
+    <Reference Include="System" />
+    <Reference Include="Microsoft.CSharp" />
+  </ItemGroup>


For something like this, it might be more convenient to set it up with a negative conditional expression ...

<ItemGroup Condition=" '$(TargetFramework)' != 'netstandard1.6' "> <Reference Include="System.Collections.Concurrent" /> <Reference Include="System" /> <Reference Include="Microsoft.CSharp" /> </ItemGroup>

... if the net45 and net451 remain. Then, you only need this one ItemGroup.

guardrex · 2017-03-23T06:35:11Z

samples/framework/libraries/migrate-library-csproj/tests/Car.Tests/Car.Tests.csproj

+
+  <ItemGroup>
+    <Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
+  </ItemGroup>


All of the comments above also apply to this file.

guardrex · 2017-03-23T06:43:45Z

Hi @conniey Let me know if any of my remarks don't make sense ... I speak "dinosaur" sometimes. 😀

conniey · 2017-03-23T06:44:55Z

I love your thoroughness. 👍 I'll start looking through these. :)

svick · 2017-03-25T15:42:34Z

In project.csproj.png, shouldn't Car.Tests be Car.Tests.csproj?

guardrex · 2017-03-25T16:28:03Z

Also along those lines regarding the folder icons, are they from one of the official sets? I show this one from the CnE_GeneralSymbols and CnE_Enterprise sets ...

conniey · 2017-04-06T04:06:47Z

@guardrex hey, I fixed all the issues you mentioned for the PR. Does this look alright now?

guardrex · 2017-04-06T04:12:51Z

@conniey Excellent. I'm headed off to 💤 get some sleep 🛌 ... I'm a few time zones behind you ... but I'll give it a look first thing in the morning.

guardrex · 2017-04-06T17:40:59Z

docs/core/porting/project-structure.md

 author: conniey
 ms.author: mairaw
-ms.date: 07/18/2016
+ms.date: 03/21/2017


Update to today's date 04/06/2017.

guardrex · 2017-04-06T17:50:55Z

@conniey Awesome ... just update the ms.date and we're good to go! 🚀

conniey · 2017-04-06T23:44:48Z

@guardrex Updated! 👍

guardrex · 2017-04-07T00:42:22Z

Thanks @conniey and to @svick!

dnfclas added the cla-already-signed label Mar 21, 2017

guardrex requested review from guardrex and removed request for guardrex March 22, 2017 22:42

guardrex suggested changes Mar 23, 2017

View reviewed changes

guardrex added Goal - Update from project.json based projects to MSBuild based projects labels Mar 23, 2017

guardrex added this to the March 2017 milestone Mar 23, 2017

guardrex assigned conniey Mar 23, 2017

mairaw removed this from the March 2017 milestone Mar 29, 2017

conniey added 10 commits April 5, 2017 16:26

Remove option for PCL

296a2c9

Update migrate project to .NET Core csproj VS2017

527d1ca

Fixes #1680. Update csproj with existing project to VS2017

1d5f3d7

CR: Update title casing and description

e9d4f57

CR: Replace double space after period with single space.

4dc84ce

CR: Fixing sentences, casing, etc.

a8f1590

CR: Update NuGet packages. Fix csproj properties

b23a137

CR: Merge ItemGroups into net45 and net461

98ec36d

CR: Updating images

6e4af07

CR: Update to net452

5b31a03

guardrex suggested changes Apr 6, 2017

View reviewed changes

Update ms.date

83a8376

guardrex approved these changes Apr 7, 2017

View reviewed changes

guardrex merged commit 9fd4e03 into dotnet:master Apr 7, 2017

conniey deleted the issue1680 branch April 16, 2017 01:08

		@@ -4,7 +4,7 @@ description: Organizing Your Project to Support .NET Framework and .NET Core
		keywords: .NET, .NET Core

		@@ -13,38 +13,24 @@ ms.assetid: 3af62252-1dfa-4336-8d2f-5cfdb57d7724

		# Organizing Your Project to Support .NET Framework and .NET Core

Update .NET Core porting document to VS2017 #1768

Update .NET Core porting document to VS2017 #1768

Uh oh!

Conversation

conniey commented Mar 21, 2017

Update .NET Core porting document to VS2017

Summary

Details

Suggested Reviewers

Uh oh!

mairaw commented Mar 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guardrex Mar 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guardrex commented Mar 23, 2017

Uh oh!

conniey commented Mar 23, 2017

Uh oh!

svick commented Mar 25, 2017

Uh oh!

guardrex commented Mar 25, 2017

Uh oh!

conniey commented Apr 6, 2017

Uh oh!

guardrex commented Apr 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guardrex commented Apr 6, 2017

Uh oh!

conniey commented Apr 6, 2017

Uh oh!

guardrex commented Apr 7, 2017

Uh oh!

Uh oh!

guardrex Mar 23, 2017 •

edited

Loading