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

Merge Path and Operation parameters #972

Merged
merged 7 commits into from
Mar 24, 2017

Conversation

saharj
Copy link
Contributor

@saharj saharj commented Mar 22, 2017

Fixes #965

@saharj saharj changed the title WIP: Merge Path and Operation parameters Merge Path and Operation parameters Mar 24, 2017
@saharj
Copy link
Contributor Author

saharj commented Mar 24, 2017

@ponelat

src/execute.js Outdated
// Add values to request
arrayOrEmpty(operation.parameters).forEach((parameter) => {
arrayOrEmpty(operation.parameters) // operation parameters
.concat(arrayOrEmpty(spec.paths[pathName].parameters)) // path parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add indent to this and the next line

}
}

const req = buildRequest({ spec, operationId: 'getPetsById', parameters: { id: 123, test: 567 } })
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove empty space inside {}.

@@ -60,6 +60,9 @@ export function eachOperation(spec, cb, find) {
// Iterate over the spec, collecting operations
for (const pathName in paths) {
for (const method in paths[pathName]) {
if (method.toUpperCase() === 'PARAMETERS') {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this check for? Would be great to add a comment clarifying the intention.

Copy link
Contributor Author

@saharj saharj Mar 24, 2017

Choose a reason for hiding this comment

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

@buunguyen Oddly, when path-parameters come before the operation like below, the method becomes PARAMETERS (here it should be GET), and that causes problems.

paths: {
  '/pet/{id}': {
    parameters: [
      {...}
    ],
    get: {
      operationId: 'getPetsById',
      parameters: [
        {...}
      ],
    }
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. The loop iterates the keys of each path, so "parameters" will appear. Doesn't matter if it appears before or after the "get".

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