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

Change all path extractors to use structs #797

Merged
merged 1 commit into from
Feb 3, 2023

Conversation

svix-andor
Copy link
Contributor

This has 2 benefits:

  1. It is a little safer because tuple params are extracted based simply on position so it's possible to make a mistake simply by putting the types in the wrong order. It still doesn't check that the path placeholders match the extractors at compile-time, though.
  2. Aide can match field names to URL placeholder names this way, as a result including the path parameters in the OpenAPI params section.

@svix-andor svix-andor force-pushed the andor/switch-to-struct-path-params branch 2 times, most recently from 830ac8e to df9be87 Compare February 2, 2023 13:18
This has 2 benefits:
1. It is a little safer because tuple params are extracted based simply
   on position so it's possible to make a mistake simply by putting the
   types in the wrong order.
2. Aide can match field names to URL placeholder names this way, as a
   result including the path parameters in the OpenAPI params section.
@svix-andor svix-andor force-pushed the andor/switch-to-struct-path-params branch from df9be87 to 27ea505 Compare February 2, 2023 13:22
Copy link
Member

@tasn tasn left a comment

Choose a reason for hiding this comment

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

TBH, it looks much better than the tuples!

@svix-andor svix-andor merged commit 40ed47a into main Feb 3, 2023
@svix-andor svix-andor deleted the andor/switch-to-struct-path-params branch February 3, 2023 10:42
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.

2 participants