-
Notifications
You must be signed in to change notification settings - Fork 55
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
#173: Deprecating endpoint mapping in favor of plain middleware #174
base: main
Are you sure you want to change the base?
Conversation
@VisualBean @yurvon-screamo are either of you familiar with aspnetcore? I haven't kept up to date with the recent changes to endpoints vs middleware. |
AsyncApiSchema.v2.AsyncApiDocument asyncApiSchema, | ||
IAsyncApiDocumentSerializer asyncApiDocumentSerializer) | ||
{ | ||
var asyncApiSchemaJson = asyncApiDocumentSerializer.Serialize(asyncApiSchema); |
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.
This could be cached instead of serialising every time
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.
My apologies but that's what's been used all the time. That's not what this PR is about. It's about enabling others to use plain middlewares in order to be less limited.
That being said, thanks for your review.
Middleware structure looks fine. This could definitely also use some integration test. If we don't have the we application factory thing yet, we can get that added to the to-do |
I disagree with internal sealed middleware. Overloading or single usage are real scenarios. |
I agree with @VisualBean that the project would benefit from caching but let's track that via: #198 |
see #173