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

Optional paths #275

Merged
merged 5 commits into from
Sep 4, 2023
Merged

Conversation

noamalffasy
Copy link
Contributor

Closes #190.

Makes paths optional.

Also removing validation checks for having at least one path (I'm actually on the fence about it because one could want to check for them, could be brought back in the case of them being needed).

Sets `paths` only if it is specified when decoded, and encodes it when it is not empty.
Also adds simple tests (based on the existing webhooks ones) that make sure the behavior works as expected.
Some of the tests are no longer necessary due to paths being optional (so for example, the validation that makes sure there's at least one path is removed).
Copy link
Owner

@mattpolzin mattpolzin left a comment

Choose a reason for hiding this comment

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

This looks great, but let's keep the validation around. It was off-by-default anyway which means it doesn't get in anyone's way but if someone does want to ensure their API docs always have paths then the validation is available out of box (it's one that I apply currently to my workplace's API documentation).

The docs for the path validation could be updated, though, now that there's a very reasonable use case that would result in no path items.

@mattpolzin
Copy link
Owner

I fixed the merge conflict that was just introduced upstream and went ahead and threw the optional validation back in because that's the only request I had before moving forward with this PR.

Copy link
Owner

@mattpolzin mattpolzin left a comment

Choose a reason for hiding this comment

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

This is ready to go. Thanks for fixing path optionality for OAS 3.1!

@mattpolzin mattpolzin merged commit 0cdd48c into mattpolzin:release/3_0 Sep 4, 2023
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants