Remove code migrated to aspnetcore #1933

JunTaoLuo · 2020-05-26T19:14:22Z

TODO after dotnet/aspnetcore#21581 is merged:

Fixup sln files
Invert darc subscription
Ingest packages from aspnetcore

NTaylorMullen

Need to also migrate the benchmarks and syntax generator:

eng/Versions.props

JunTaoLuo · 2020-05-26T21:08:40Z

I wasn't aware those need to be migrated. I'll handle those in a mop up.

JunTaoLuo · 2020-05-27T06:17:18Z

@NTaylorMullen I can move RazorSyntaxGenerator but the benchmarks project depends on Microsoft.CodeAnalysis.Razor.Workspaces. Does it mean that we need to move both of those?

JunTaoLuo · 2020-05-27T06:23:02Z

Also, where should the benchmarks move to in aspnetcore? Based on the naming, it would probably be src/Razor/Razor/perf/ but the rest of the projects under src/Razor/Razor were already in aspnetcore and doesn't have much to do with the benchmarks project, so I think src/Razor/perf might make more sense. Thoughts?

...Razor/src/Microsoft.VisualStudio.Mac.RazorAddin/Microsoft.VisualStudio.Mac.RazorAddin.csproj

...osoft.VisualStudio.LanguageServices.Razor/ProjectSystem/Rules/RazorGenerateWithTargetPath.cs

NTaylorMullen · 2020-05-27T17:30:28Z

@NTaylorMullen I can move RazorSyntaxGenerator but the benchmarks project depends on Microsoft.CodeAnalysis.Razor.Workspaces. Does it mean that we need to move both of those?

Ahh, we may need to split the benchmarks into two. One for aspnetcore and one for aspnetcore-tooling. I don't want to have Microsoft.CodeAnalysis.Razor.Workspaces in aspnetcore. Currently the benchmarks only has a single file that depends on the Razor.Workspaces project so everything else can go.

Also, where should the benchmarks move to in aspnetcore? Based on the naming, it would probably be src/Razor/Razor/perf/ but the rest of the projects under src/Razor/Razor were already in aspnetcore and doesn't have much to do with the benchmarks project, so I think src/Razor/perf might make more sense. Thoughts?

Makes perfect sense to me!

Hmm seems like pdbs are not included in nupkgs.

We only include the pdbs in the mpacks which might prove to a point of concern in this migration.

JunTaoLuo · 2020-05-28T23:29:14Z

I'll need to ingest newer packages and pdbs from aspnetcore when they are available, but otherwise the PR is ready for review.

dougbu

To finish this off, suggest running `darc update-dependencies --source-repo dotnet/aspnetcore --channel '.NET 5 Dev' while in this branch locally. If that doesn't do what you expect, fix it 😺 Otherwise, you've preemptively done the first Maestro++ update for the flow you can add after this PR is merged.

eng/Version.Details.xml

eng/Versions.props

src/Razor/Razor.NoVsix.sln

src/Razor/src/Microsoft.VisualStudio.Editor.Razor/LegacyProjectEngineFactory_1_0.cs

...oft.VisualStudio.LanguageServices.Razor/Microsoft.VisualStudio.LanguageServices.Razor.csproj

...Razor/src/Microsoft.VisualStudio.Mac.RazorAddin/Microsoft.VisualStudio.Mac.RazorAddin.csproj

...Razor/src/Microsoft.VisualStudio.RazorExtension/Microsoft.VisualStudio.RazorExtension.csproj

eng/AfterSigning.targets

dougbu · 2020-05-29T00:04:38Z

To be clear

Invert darc subscription

Don't do this until this PR is merged. Also, delete the disabled aspnetcore-tooling -> aspnetcore edge for the 'master' branch now or at the same time. aspnetcore shouldn't need anything from here. If it does, that's a bug to fix before merging this PR.

JunTaoLuo · 2020-05-29T16:50:28Z

Needs another look @NTaylorMullen @dougbu I've addressed feedback items and updated to consume the latest aspnetcore packages.

ryanbrandenburg

I reviewed this by only looking at the modified files, assuming that the deleted files will have mirrored adds in dotnet/aspnetcore. Looks good. For a change of this magnitude I think it makes sense we wait for multiple approvals though.

ryanbrandenburg · 2020-05-29T17:39:22Z

Needs another look @NTaylorMullen @dougbu I've addressed feedback items and updated to consume the latest aspnetcore packages.
I keep forgetting it exists, but GitHub has a "request re-review" button that's nice for this kind of thing. Sometimes it still makes sense to make a comment if you need to explain what has changed though.

JunTaoLuo · 2020-05-29T17:42:39Z

Sometimes it still makes sense to make a comment if you need to explain what has changed though.

I did 😄, I've addressed feedback items and updated to consume the latest aspnetcore packages.

eng/Versions.props

NTaylorMullen · 2020-05-29T18:25:58Z

Oh actually @JunTaoLuo you should remove the testapps folder:

And update the .vscode/settings.json file to not boot to it

TanayParikh

I reviewed this by only looking at the modified files, assuming that the deleted files will have mirrored adds in dotnet/aspnetcore. Looks good. For a change of this magnitude I think it makes sense we wait for multiple approvals though.

+1

JunTaoLuo · 2020-05-29T18:30:39Z

@NTaylorMullen I was under the impression a lot of the apps are still used elsewhere e.g. https://github.com/dotnet/aspnetcore-tooling/blob/master/src/Razor/test/VSCode.FunctionalTest/tests/TestUtil.ts#L15-L19 so I elected to keep all the tests apps. I can do a more granular check and remove ones that are not referenced I suppose.

NTaylorMullen · 2020-05-29T18:35:43Z

@NTaylorMullen I was under the impression a lot of the apps are still used elsewhere e.g. https://github.com/dotnet/aspnetcore-tooling/blob/master/src/Razor/test/VSCode.FunctionalTest/tests/TestUtil.ts#L15-L19 so I elected to keep all the tests apps. I can do a more granular check and remove ones that are not referenced I suppose.

Ooh ya look at that. You've learned more about the tooling repo now then I have 😆

dougbu · 2020-05-29T20:03:53Z

eng/Version.Details.xml

+      <Uri>https://github.com/dotnet/aspnetcore</Uri>
+      <Sha>a799bf68e45c63e62060a7916ebb6d4f87a9aa1b</Sha>
+    </Dependency>
+    <Dependency Name="Microsoft.Extensions.Configuration.Json" Version="5.0.0-preview.6.20277.12" CoherentParentDependency="Microsoft.CodeAnalysis.Razor">


This is very weird and is not coherent with the dotnet/aspnetcore 5.0.0-preview.6.20278.12 build. We also don't want to downgrade these dependencies. Suspect you need to run darc update-dependencies --coherency-only after we get dotnet/aspnetcore#1947 in.

@mmitche @wtgodbe the dotnet/aspnetcore is using dotnet/runtime dependencies at 5.0.0-preview.6.20271.10 at the a799bf6 commit. Why didn't CoherentParentDependency do the "correct" thing and back up all that way? We don't really want to downgrade at all but that at least would have been coherent.

It did the expected thing when I ran it.

You mean it downgraded to 5.0.0-preview.6.20271.10?

Oops, I pointed to the wrong dependency update PR.

@JunTaoLuo only just added an arc updating dotnet/aspnetcore directly from dotnet/runtime. I triggered it moments ago. This PR won't be ready to go (really, the dependencies won't be coherent without downgrading) until dotnet/aspnetcore#22370 and the subsequent official build and Maestro++ update are done.

@JunTaoLuo you've got lots of approval here but please don't merge until you're able to update the dependencies in this branch correctly.

Yep, it downgraded locally. What's the darc version that got used to do that update? Seems unlikely that would affect things but could be.

Locally for me, darc doesn't seem to do any coherency updates, but it could be because aspnetcore hasn't ingested newer runtime versions yet. I'm waiting to confirm that once the runtime update in aspnetcore is merged.

Triggered a subscription update and looks like the latest commit shows that updates are working as expected.

Also remove RazorPageGenerator

- remove CodeAnalysis.Razor.Tests - remove source build step

…0200529.12 (#1951) Microsoft.NET.Sdk.Razor , Microsoft.CodeAnalysis.Razor , Microsoft.AspNetCore.Razor.Internal.Transport , Microsoft.AspNetCore.Razor.Language , Microsoft.AspNetCore.Mvc.Razor.Extensions.Version2_X , Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X , Microsoft.AspNetCore.Mvc.Razor.Extensions From Version 5.0.0-preview.6.20278.12 -> To Version 5.0.0-preview.6.20279.12 Dependency coherency updates Microsoft.Extensions.Configuration.Json,Microsoft.Extensions.Logging,System.Diagnostics.DiagnosticSource,System.Resources.Extensions,System.Text.Encodings.Web,Microsoft.Extensions.DependencyModel,Microsoft.NETCore.App.Ref,Microsoft.NETCore.App.Internal,Microsoft.NETCore.App.Runtime.win-x64,Microsoft.NETCore.Platforms From Version 5.0.0-preview.6.20277.12 -> To Version 5.0.0-preview.6.20278.9 (parent: Microsoft.CodeAnalysis.Razor Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

NTaylorMullen · 2020-05-30T17:00:23Z

Woooo!!

NTaylorMullen suggested changes May 26, 2020

View reviewed changes

eng/Versions.props Show resolved Hide resolved

JunTaoLuo commented May 27, 2020

View reviewed changes

...Razor/src/Microsoft.VisualStudio.Mac.RazorAddin/Microsoft.VisualStudio.Mac.RazorAddin.csproj Outdated Show resolved Hide resolved

JunTaoLuo commented May 27, 2020

View reviewed changes

...osoft.VisualStudio.LanguageServices.Razor/ProjectSystem/Rules/RazorGenerateWithTargetPath.cs Outdated Show resolved Hide resolved

JunTaoLuo mentioned this pull request May 27, 2020

Johluo/migrate more tooling dotnet/aspnetcore#22294

Merged

JunTaoLuo force-pushed the johluo/remove-migrated-projects branch from 8ea6d87 to 4707905 Compare May 28, 2020 00:19

JunTaoLuo mentioned this pull request May 28, 2020

Add a transport package for Mvc.Razor.Extensions pdbs dotnet/aspnetcore#22299

Merged

JunTaoLuo marked this pull request as ready for review May 28, 2020 23:28

JunTaoLuo requested review from ajaybhargavb, dougbu, javiercn, noahbkim, pranavkm, ryanbrandenburg and TanayParikh as code owners May 28, 2020 23:28

dougbu reviewed May 29, 2020

View reviewed changes

JunTaoLuo force-pushed the johluo/remove-migrated-projects branch from da7ec3d to de9454c Compare May 29, 2020 09:42

ryanbrandenburg approved these changes May 29, 2020

View reviewed changes

JunTaoLuo requested review from NTaylorMullen and dougbu May 29, 2020 17:42

NTaylorMullen approved these changes May 29, 2020

View reviewed changes

eng/Versions.props Show resolved Hide resolved

TanayParikh approved these changes May 29, 2020

View reviewed changes

JunTaoLuo force-pushed the johluo/remove-migrated-projects branch from b3aed5b to ef0ff38 Compare May 29, 2020 19:45

dougbu approved these changes May 29, 2020

View reviewed changes

John Luo and others added 14 commits May 29, 2020 16:54

Remove code migrated to aspnetcore

7883b26

Also remove RazorPageGenerator

Reference migrated projects as packages

dea15b6

Fix Assembly.Load test failures.

93e22ba

Rever unintended changes

0948f7b

- remove CodeAnalysis.Razor.Tests - remove source build step

Remove migrated benchmarks

c566014

Remove RazorSyntaxGenerator

0c1c67b

Feedback

8b52a9c

Rebase updates

dd4b65b

Use internal transport

8047839

Update dependencies via darc

11ac17a

Typo

bb9181a

Try using tooling CA.CSharp version

3bb71af

Remove unused test aspps

af4d028

Remove redundant sln file

18ac899

JunTaoLuo force-pushed the johluo/remove-migrated-projects branch 2 times, most recently from de22ef7 to 18ac899 Compare May 30, 2020 08:25

JunTaoLuo merged commit f2fab7a into master May 30, 2020

JunTaoLuo deleted the johluo/remove-migrated-projects branch May 30, 2020 09:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove code migrated to aspnetcore #1933

Remove code migrated to aspnetcore #1933

JunTaoLuo commented May 26, 2020 •

edited

Loading

NTaylorMullen left a comment

JunTaoLuo commented May 26, 2020

JunTaoLuo commented May 27, 2020

JunTaoLuo commented May 27, 2020 •

edited

Loading

NTaylorMullen commented May 27, 2020

JunTaoLuo commented May 28, 2020

dougbu left a comment

dougbu commented May 29, 2020

JunTaoLuo commented May 29, 2020

ryanbrandenburg left a comment •

edited

Loading

ryanbrandenburg commented May 29, 2020

JunTaoLuo commented May 29, 2020

NTaylorMullen commented May 29, 2020

TanayParikh left a comment

JunTaoLuo commented May 29, 2020

NTaylorMullen commented May 29, 2020

dougbu May 29, 2020

mmitche May 29, 2020

dougbu May 29, 2020

dougbu May 29, 2020

dougbu May 29, 2020

mmitche May 29, 2020

JunTaoLuo May 29, 2020

JunTaoLuo May 30, 2020

NTaylorMullen commented May 30, 2020

Remove code migrated to aspnetcore #1933

Remove code migrated to aspnetcore #1933

Conversation

JunTaoLuo commented May 26, 2020 • edited Loading

NTaylorMullen left a comment

Choose a reason for hiding this comment

JunTaoLuo commented May 26, 2020

JunTaoLuo commented May 27, 2020

JunTaoLuo commented May 27, 2020 • edited Loading

NTaylorMullen commented May 27, 2020

JunTaoLuo commented May 28, 2020

dougbu left a comment

Choose a reason for hiding this comment

dougbu commented May 29, 2020

JunTaoLuo commented May 29, 2020

ryanbrandenburg left a comment • edited Loading

Choose a reason for hiding this comment

ryanbrandenburg commented May 29, 2020

JunTaoLuo commented May 29, 2020

NTaylorMullen commented May 29, 2020

TanayParikh left a comment

Choose a reason for hiding this comment

JunTaoLuo commented May 29, 2020

NTaylorMullen commented May 29, 2020

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

NTaylorMullen commented May 30, 2020

JunTaoLuo commented May 26, 2020 •

edited

Loading

JunTaoLuo commented May 27, 2020 •

edited

Loading

ryanbrandenburg left a comment •

edited

Loading