React to Razor.Design package removal #8680

pranavkm · 2018-10-31T18:19:22Z

Remove references to packages (Razor.Design, Razor.Extensions) solely used to bring in compiler \ targets
Target netcoreapp3.0 in samples and tests to allow Sdk to infer values

pranavkm · 2018-10-31T18:21:40Z

...Microsoft.Extensions.ApiDescription.Design/Microsoft.Extensions.ApiDescription.Design.csproj

      <PackagePath>tools/dotnet-getdocument.dll</PackagePath>
      <StrongName>$(AssemblySigningStrongName)</StrongName>
    </SignedPackageFile>
+    <SignedPackageFile Include="../dotnet-getdocument/bin/$(Configuration)/netcoreapp2.1/publish/dotnet-getdocument.exe">


@dougbu the build produces an extra exe in the publish dir. Filing #8681 to investigate this later.

Sounds good 😺

pranavkm · 2018-10-31T18:21:57Z

build/dependencies.props

-    <MicrosoftNETSdkRazorPackageVersion>3.0.0-alpha1-10654</MicrosoftNETSdkRazorPackageVersion>
+    <MicrosoftNETCoreApp30PackageVersion>3.0.0-preview1-26907-05</MicrosoftNETCoreApp30PackageVersion>
+    <MicrosoftNetHttpHeadersPackageVersion>3.0.0-alpha1-10670</MicrosoftNetHttpHeadersPackageVersion>
+    <MicrosoftNETSdkRazorPackageVersion>3.0.0-a-alpha1-sdk-31823</MicrosoftNETSdkRazorPackageVersion>


Feature branch of Razor Sdk

dougbu · 2018-10-31T18:50:31Z

@pranavkm it's possible .NET Core changed their default formats between 2.2 and 3.0. Since the failure doesn't occur without your changes, I suspect that's the case. The line to change is

Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/RouteValueProviderTests.cs

Line 78 in c74a945

    
           var expected = TestPlatformHelper.IsWindows ? "31.10.2018 07:37:38 -07:00" : "31.10.18 07:37:38 -07:00";

dougbu · 2018-10-31T19:03:33Z

@pranavkm I found the relevant CoreFx change: See issue dotnet/corefx#29538, PR dotnet/coreclr#18316 or commit dotnet/coreclr@a51328304a. Their change is in 3.0 only.

dougbu

mostly questions and suggestions…

dougbu · 2018-10-31T19:05:46Z

benchmarkapps/BasicApi/BasicApi.csproj

 <Project Sdk="Microsoft.NET.Sdk.Web">
  <PropertyGroup>
-    <TargetFrameworks>netcoreapp2.2</TargetFrameworks>
+    <TargetFrameworks>netcoreapp3.0</TargetFrameworks>


What fails if we use $(StandardTestWebsiteTfms) in these projects with hard-coded target frameworks? If we can, would be great to get rid of the special cases.

The variable isn't defined in the root Directory.Build.props (it's in the test Directory.Build.props). So wouldn't be available here. In addition, I think these projects are meant to be buildable standalone on the benchmark server, so I'm going to leave it be for now

Ah, now I remember doing that…

dougbu · 2018-10-31T19:07:20Z

test/WebSites/RazorPagesClassLibrary/RazorPagesClassLibrary.csproj

+
+    <_EnableAllInclusiveRazorSdk>true</_EnableAllInclusiveRazorSdk>
+    <RazorLangVersion>2.1</RazorLangVersion>
+    <RazorDefaultConfiguration>MVC-2.1</RazorDefaultConfiguration>


Why target only 2.1 in this project?

It's the most recent version of Razor that's available

dougbu · 2018-10-31T19:08:09Z

...Microsoft.Extensions.ApiDescription.Design/Microsoft.Extensions.ApiDescription.Design.csproj

      <PackagePath>tools/dotnet-getdocument.dll</PackagePath>
      <StrongName>$(AssemblySigningStrongName)</StrongName>
    </SignedPackageFile>
+    <SignedPackageFile Include="../dotnet-getdocument/bin/$(Configuration)/netcoreapp2.1/publish/dotnet-getdocument.exe">


Sounds good 😺

dougbu · 2018-10-31T19:10:36Z

build/dependencies.props

+    <MicrosoftExtensionsWebEncodersPackageVersion>3.0.0-alpha1-10670</MicrosoftExtensionsWebEncodersPackageVersion>
    <MicrosoftNETCoreApp20PackageVersion>2.0.9</MicrosoftNETCoreApp20PackageVersion>
    <MicrosoftNETCoreApp21PackageVersion>2.1.3</MicrosoftNETCoreApp21PackageVersion>
-    <MicrosoftNETCoreApp22PackageVersion>2.2.0-preview3-27014-02</MicrosoftNETCoreApp22PackageVersion>


We don't want any tests using 2.2 anymore? Seems extreme and leaves us with a testing gap.

I don't think we've ever tested an earlier version of netcoreapp TFM in our repo.

dougbu · 2018-10-31T19:12:48Z

build/dependencies.props

-    <MicrosoftAspNetCoreBenchmarkRunnerSourcesPackageVersion>3.0.0-alpha1-10654</MicrosoftAspNetCoreBenchmarkRunnerSourcesPackageVersion>
-    <MicrosoftAspNetCoreChunkingCookieManagerSourcesPackageVersion>3.0.0-alpha1-10654</MicrosoftAspNetCoreChunkingCookieManagerSourcesPackageVersion>
-    <MicrosoftAspNetCoreCookiePolicyPackageVersion>3.0.0-alpha1-10654</MicrosoftAspNetCoreCookiePolicyPackageVersion>
-    <MicrosoftAspNetCoreCorsPackageVersion>3.0.0-a-alpha1-w-m-16569</MicrosoftAspNetCoreCorsPackageVersion>


To confirm: We no longer need to use the feature branch packages for CORS or Routing?

Yeah. Haven't seen any test failures as a result of updating

* Remove references to packages (Razor.Design, Razor.Extensions) solely used to bring in compiler \ targets * Target netcoreapp3.0 in samples and tests to allow Sdk to infer values

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/RouteValueProviderTests.cs

dougbu · 2018-10-31T21:09:51Z

benchmarkapps/BasicApi/BasicApi.csproj

 <Project Sdk="Microsoft.NET.Sdk.Web">
  <PropertyGroup>
-    <TargetFrameworks>netcoreapp2.2</TargetFrameworks>
+    <TargetFrameworks>netcoreapp3.0</TargetFrameworks>


Ah, now I remember doing that…

pranavkm requested a review from rynowak October 31, 2018 18:19

pranavkm commented Oct 31, 2018

View reviewed changes

dougbu suggested changes Oct 31, 2018

View reviewed changes

React to Razor.Design package removal

5e6b7f9

* Remove references to packages (Razor.Design, Razor.Extensions) solely used to bring in compiler \ targets * Target netcoreapp3.0 in samples and tests to allow Sdk to infer values

pranavkm force-pushed the prkrishn/react-razor-design branch from 7deddc3 to 5e6b7f9 Compare October 31, 2018 19:44

pranavkm commented Oct 31, 2018

View reviewed changes

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/RouteValueProviderTests.cs Show resolved Hide resolved

pranavkm closed this Oct 31, 2018

pranavkm reopened this Oct 31, 2018

dougbu approved these changes Oct 31, 2018

View reviewed changes

Update RouteValueProviderTests.cs

a1ca733

pranavkm closed this Oct 31, 2018

pranavkm reopened this Oct 31, 2018

pranavkm merged commit 17ed44c into master Oct 31, 2018

pranavkm deleted the prkrishn/react-razor-design branch October 31, 2018 22:38

React to Razor.Design package removal #8680

React to Razor.Design package removal #8680

Uh oh!

Conversation

pranavkm commented Oct 31, 2018

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!

dougbu commented Oct 31, 2018

Uh oh!

dougbu commented Oct 31, 2018

Uh oh!

dougbu left a comment

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!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants