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

Add path param support #56

Merged
merged 2 commits into from
Jan 19, 2018
Merged

Conversation

jessealva
Copy link
Contributor

No description provided.

@jessealva
Copy link
Contributor Author

@mdeuser could you review this real quick? its a change to enable parameters as children of Paths

@dubee
Copy link
Member

dubee commented Jan 9, 2018

@jessealva, can we unit test this?

whisk/api.go Outdated
@@ -146,13 +146,51 @@ type ApiSwagger struct {
SwaggerName string `json:"swagger,omitempty"`
BasePath string `json:"basePath,omitempty"`
Info *ApiSwaggerInfo `json:"info,omitempty"`
Paths map[string]map[string]*ApiSwaggerOperation `json:"paths,omitempty"` // Paths["/a/path"]["get"] -> a generic object
Paths map[string]*ApiSwaggerPath `json:"paths,omitempty"` // Paths["/a/path"]["get"] -> a generic object
Copy link
Contributor

Choose a reason for hiding this comment

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

i think the comment is incorrect. should be something like
// Paths["/a/path"] -> a path (ApiSwaggerPath) object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the comment as I think its no longer required as it was there when the object was a map of maps.

@jessealva
Copy link
Contributor Author

@dubeejw So unless I'm mistaken there is no testing infrastructure setup already? I'm afraid if I attempt to test this then it will prevent me from actually getting this feature in this month. Looks like we already have a feature request for this #14 . Could we just do this testing at that point?

@dubee
Copy link
Member

dubee commented Jan 19, 2018

@jessealva, looks great! Do you have tests for this at least in incubator-openwhisk-cli?

@dubee dubee added the api gw label Jan 19, 2018
@dubee dubee self-assigned this Jan 19, 2018
@dubee dubee merged commit a086445 into apache:master Jan 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants