Skip to content
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

Conversation

ch-ti8m-michalpenka
Copy link

see #173

@m-wild
Copy link
Collaborator

m-wild commented Mar 8, 2024

@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);
Copy link
Collaborator

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

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.

@VisualBean
Copy link
Collaborator

VisualBean commented Mar 9, 2024

@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.

Middleware structure looks fine.
My only comments on this is around caching of values. I'll take a deeper look Monday though.

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

@yurvon-screamo
Copy link
Collaborator

I disagree with internal sealed middleware.

Overloading or single usage are real scenarios.

@thompson-tomo
Copy link

I agree with @VisualBean that the project would benefit from caching but let's track that via: #198

@yurvon-screamo
Copy link
Collaborator

due to the lack of understanding of the benefits to the project from this change and any activities related to it, I close the request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants