-
Notifications
You must be signed in to change notification settings - Fork 530
Open API 3.1.0 support #1768
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
base: master
Are you sure you want to change the base?
Open API 3.1.0 support #1768
Conversation
This involved breaking 2 and 3 out a little to better mirror the differences in the specs
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.
Hello there jefflembeck 👋
Thank you and congrats 🎉 for opening your first PR on this project.✨
We will review the following PR soon! 👀
|
My real issue I wanted to get to the bottom of was getting variadic tuples in. But the differences in |
2d4e2e2 to
83aaccb
Compare
|
Any news? |
|
I started on this, but I am very much beyond my capacity until October unfortunately. |
| import { DEFAULT_REQUEST_MEDIA_TYPE, DEFAULT_RESPONSE_MEDIA_TYPE, getValue } from './../utils/swaggerUtils'; | ||
| import { SpecGenerator } from './specGenerator'; | ||
|
|
||
| export class SpecGenerator31 extends SpecGenerator { |
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.
First glance, this is probably better extending SpecGenerator3, have you considered that option?
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.
I did. This was actually something I wanted to talk about a little. This part fully got away from me. Differences between examples was my start.
| }, | ||
| err => { | ||
| expect(err.message).to.equal('Unsupported Spec version.'); | ||
| expect(err.message).to.equal('Unsupported Spec version: -2.'); |
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.
Why -2
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.
Oh, that's just me outputting the spec version that is wrong. (That's the -2 from above)
I made this change while I was working because it made a few things easier to figure out.
| if (swaggerConfig.specVersion) { | ||
| if (swaggerConfig.specVersion === 3) { | ||
| spec = new SpecGenerator3(metadata, swaggerConfig).GetSpec(); | ||
| } else if (swaggerConfig.specVersion === 3.1) { | ||
| spec = new SpecGenerator31(metadata, swaggerConfig).GetSpec(); | ||
| } else { | ||
| spec = new SpecGenerator2(metadata, swaggerConfig).GetSpec(); | ||
| } | ||
| } else { | ||
| spec = new SpecGenerator2(metadata, swaggerConfig).GetSpec(); | ||
| } |
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.
I am fine with making 3.1 a default in the scope of a major release
All Submissions:
Closing issues
This addresses number #794.
Put
closes #XXXX(where XXXX is the issue number) in your comment to auto-close the issue that your PR fixes.Potential Problems With The Approach
There is probably way too much here. I cargo culted a ton of things over from 3.0.0. I also made some changes to how certain types extend others.
Test plan
Broke out tests and fixtures strictly for 3.1. Happy to make changes to that, but wanted something we could start talking out earlier rather than later.