-
Notifications
You must be signed in to change notification settings - Fork 266
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
Add ability to Configure Endpoints via IEndpointConventionBuilder #599
Conversation
Hello, thank you for this PR! I can't review the code as I'm not suited enough for that. |
Hi there, Yes, I would be fine with it being part of the organization. |
Hi, thanks for the PR, seems pretty cool. I think I'll have some time to review it next week! |
Fantastic, looking at it I don't see any issues. Since this works it would mean a lot to many of us if this was merged. |
Thanks for submitting this RP. I think this looks really great 😊 We should also update the documentation before merging. I'm trying to figure out if this is a breaking change or not i.e for applications upgrading Giraffe to a newer version of the library. The val addMetadata:
metadata: obj ->
endpoint: Endpoint
-> Endpoint to val addMetadata:
metadata: obj
-> Endpoint -> Endpoint I don't think it's a breaking change 🙈 but I'll let someone who knows F# better than me decide if it makes sense to keep the exact same signature just to be sure i.e change to: let addMetadata (metadata: obj) (endpoint: Endpoint)=
endpoint |> configureEndpoint _.WithMetadata(metadata) |
I've updated the |
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 can add the documentation later with another PR.
Btw, what I have in mind is:
- Some paragraphs explaining how to use this new feature;
- A new sample project.
Thanks for the review @64J0. Can we ship this as an alpha? |
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 is great, and should be a pretty safe change since there's no breaking change and all the configure endpoint stuff is ASP specific anyways. The PR just enables you to configure it.
Description
This PR adds the ability to configure endpoints via the IEndpointConventionBuilder. This would enable us to reuse some of the built-in OpenApi generation to generate OpenApi schemas from code. It's possible by replacing the
MetadataList
with aConfigureEndpoint
, which can be utilized to capture additional information about the endpoint.I've taken inspiration from Oxpeckers Routing which uses a similar approach.
How to test
A working example provided in the Giraffe.OpenApi repo, which utilizes this branch. It's based on the MinimalApi approach, with a more functional syntax.
In the example Swagger has been used to display the configured endpoints.
Endpoints can be added to the OpenApi documentation as follows:
We have the type
FsharpMessage
, with XML-documentation:and the endpoints
/hello
which will be produce the view below:
Endpoint and Type in Swagger
And for extra references the whole app.
Swagger view of the endpoints.
If it's accepted, I'll either publish
Giraffe.OpenApi
or make another MR for it here.Related issues