Skip to content

Trim Microsoft.AspNetCore #42555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 7, 2022
Merged

Trim Microsoft.AspNetCore #42555

merged 2 commits into from
Jul 7, 2022

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Jul 3, 2022

No description provided.

@JamesNK JamesNK requested review from Tratcher, halter73, a team, dougbu, wtgodbe and Pilchie as code owners July 3, 2022 14:21
@JamesNK JamesNK requested a review from davidfowl July 6, 2022 13:30
@JamesNK
Copy link
Member Author

JamesNK commented Jul 6, 2022

Review please 🙏

@@ -173,7 +173,7 @@ IWebHostBuilder ISupportsStartup.Configure(Action<WebHostBuilderContext, IApplic
throw new NotSupportedException("Configure() is not supported by WebApplicationBuilder.WebHost. Use the WebApplication returned by WebApplicationBuilder.Build() instead.");
}

IWebHostBuilder ISupportsStartup.UseStartup(Type startupType)
IWebHostBuilder ISupportsStartup.UseStartup([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicMethods)] Type startupType)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this a constant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go the other way and just delete StartupLinkerOptions. This is simple enough I like just seeing the flags defined inline.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? I guess this will never change so its not a big deal.

@@ -173,7 +173,7 @@ IWebHostBuilder ISupportsStartup.Configure(Action<WebHostBuilderContext, IApplic
throw new NotSupportedException("Configure() is not supported by WebApplicationBuilder.WebHost. Use the WebApplication returned by WebApplicationBuilder.Build() instead.");
}

IWebHostBuilder ISupportsStartup.UseStartup(Type startupType)
IWebHostBuilder ISupportsStartup.UseStartup([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicMethods)] Type startupType)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go the other way and just delete StartupLinkerOptions. This is simple enough I like just seeing the flags defined inline.

@JamesNK JamesNK force-pushed the jamesnk/defaultbuilder branch from 49eeea8 to a2db098 Compare July 7, 2022 01:42
@JamesNK JamesNK enabled auto-merge (squash) July 7, 2022 03:03
@AraHaan
Copy link
Member

AraHaan commented Jul 7, 2022

So, what all gets trimmed with this?

@JamesNK JamesNK merged commit c4efbfb into main Jul 7, 2022
@JamesNK JamesNK deleted the jamesnk/defaultbuilder branch July 7, 2022 03:33
@ghost ghost added this to the 7.0-preview7 milestone Jul 7, 2022
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants