-
Notifications
You must be signed in to change notification settings - Fork 63
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
Update remote app builder pattern #120
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. As I looked at the changes to samples, though, it occurred to me that we might be able to simplify the API a little by having AddRemoteAppServer
and AddRemoteAppClient
configure different interfaces (ISystemWebAdapterRemoteAppClientBuilder
and ISystemWebAdapterRemoteAppServerBuilder
). Then the extension methods that work on those builders wouldn't all need the "client" and "server" designations (since they would extend different interfaces) and overall usage would simplify a little.
src/Microsoft.AspNetCore.SystemWebAdapters/Microsoft.AspNetCore.SystemWebAdapters.csproj
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/ISystemWebAdapterRemoteAppBuilder.Shared.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/RemoteAppServerExtensions.Framework.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll cover this in detail in the next API review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. We should update docs to reflect the changes, too. If you want to do that in a separate PR, though, this part looks good.
:) I was in the process of updating the docs so I wouldn't forget |
PR Title
Update builder pattern for remote to clearly separate client and server scenarios
PR Description
This change makes a few API changes we've discussed around being clear around remote app options and extension methods. This introduces a new builder API (ISystemWebAdapterRemoteAppBuilder) that allows for a nested configuration pattern similar to the following:
ASP.NET Core:
ASP.NET Framework:
This change also separates out the options for Client/Server to be separate classes
Part of #112