Use attributes for application part discovery #10271
Conversation
pranavkm
force-pushed
the
prkrishn/mvc-discovery
branch
from
f51658b to
c737497
Compare
Eilon
added
the
area-mvc
pranavkm
force-pushed
the
prkrishn/mvc-discovery
branch
from
c737497 to
93d14ae
Compare
| @@ -15,7 +15,6 @@ | |||
| <Reference Include="Microsoft.AspNetCore.Routing.Abstractions" /> | |||
| <Reference Include="Microsoft.AspNetCore.Routing" /> | |||
| <Reference Include="Microsoft.Extensions.DependencyInjection" /> | |||
| <Reference Include="Microsoft.Extensions.DependencyModel" /> | |||
There was a problem hiding this comment.
@natemcmaster we could remove Microsoft.Extensions.DependencyModel from the shared fx now. None of the other MVC assemblies that reference it are part of Microsoft.AspNetCore.App. Should I remove it?
There was a problem hiding this comment.
Let's run this by Damian and Fowler. Can you send mail on this? I'm personally okay with it, but let's make sure we think measure the impact first.
pranavkm
force-pushed
the
prkrishn/mvc-discovery
branch
from
d1a52b3 to
d7fb1bb
Compare
| @@ -56,9 +56,6 @@ | |||
| <ExternalAspNetCoreAppReference Include="System.Security.Cryptography.Xml" Version="$(SystemSecurityCryptographyXmlPackageVersion)" /> | |||
| <ExternalAspNetCoreAppReference Include="System.Threading.Channels" Version="$(SystemThreadingChannelsPackageVersion)" /> | |||
|
|
|||
| <!-- Dependencies from dotnet/core-setup --> | |||
| <ExternalAspNetCoreAppReference Include="Microsoft.Extensions.DependencyModel" Version="$(MicrosoftExtensionsDependencyModelPackageVersion)" /> | |||
There was a problem hiding this comment.
Can you update this list? #3755
|
This comment was made automatically. If there is a problem contact [email protected]. I've triaged the above build. I've created/commented on the following issue(s) |
natemcmaster
added
the
breaking-change
ghost
deleted the
|
👏 👏 |
|
@pranavkm Can/Did we turn off PreserveCompilationContext now in the Razor SDK? |
| @@ -52,18 +52,56 @@ public void PopulateFeature<TFeature>(TFeature feature) | |||
|
|
|||
| internal void PopulateDefaultParts(string entryAssemblyName) | |||
There was a problem hiding this comment.
@pranavkm it would be good to profile this to understand how this particular piece of code scales (allocations wise) with the number of assemblies. I know it only happens once per app but I'm curious mostly because of the amount of LINQ
There was a problem hiding this comment.
Ok. We used to have a ton of Linq in the Deps file code too which is primarily why I didn't think it was a big deal: https://github.com/aspnet/AspNetCore/blob/release/3.0-preview5/src/Mvc/Mvc.Core/src/ApplicationParts/ApplicationAssembliesProvider.cs#L48-L79. We could switch it to vanilla for loops if you think this might be something to be concerned about.
No description provided.