-
-
Notifications
You must be signed in to change notification settings - Fork 532
Fix missing parameters in operation object #1090
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
Conversation
ctx.operations[operationObject.operationId] = { | ||
operationType, | ||
comment: getSchemaObjectComment(operationObject, 1), | ||
}; | ||
output.push(indent(`${method}: operations[${escStr(operationObject.operationId)}];`, indentLv)); | ||
} else { | ||
// fold top-level PathItem parameters into method-level, with the latter overriding the former | ||
const keyedParameters: Record<string, ParameterObject | ReferenceObject> = {}; |
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.
Just moves this up one level; that’s it!
@@ -276,7 +276,7 @@ export interface RequestBodyObject extends Extensable { | |||
*/ | |||
export interface MediaTypeObject extends Extensable { | |||
/** The schema defining the content of the request, response, or parameter. */ | |||
schema?: SchemaObject; | |||
schema?: SchemaObject | ReferenceObject; |
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.
Irrelevant type bug that came up while writing the test
_Code generation is hard!_ And for that reason, starting with a very clear expectation of your end-goal can make working easier. | ||
Reasoning about code generation can be quite difficult until you “invert your thinking” and approach it output-first. Adopting TDD can turn very unclear/abstract problems into concrete ones with clear steps to resolution. | ||
|
||
✨ When starting any task, **write a failing test first!** ✨ |
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.
Tweaked wording of this to harp on how helpful TDD is when contributing.
I am not that smart. I need TDD to be able to do anything in this library :P
Changes
Fixes #1065. This was a dumb bug where we just didn’t do a complete parameter lookup if it had an
operationId
🤦. Was as easy as moving a couple lines of code above theif()
statement.How to Review
Captured in tests; tests should pass.
Checklist
examples/
directory updated (if applicable)