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

Return arrays instead of objects for many GETs #16

Merged
merged 2 commits into from
May 27, 2016
Merged

Conversation

codenrhoden
Copy link
Collaborator

Matching PR to RachHD is here: RackHD/on-http#292

Many of the GET schemas are wrong, listing the return as a generic object instead of a list of objects. Fix that.

Also, remove all the extra whitespace from monorail.yml in a separate commit.

Probably want to merge #15 first, and then I can rebase this one on top of it. But it can be reviewed as-is.

@kacole2
Copy link
Contributor

kacole2 commented May 23, 2016

i'm looking at this and not sure why this is being changed. What's wrong with the returned object? More parsing?

My main concern is not wanting to keep two different versions of the monorail.yml. One for this and one from the main. The monorail.yml is automatically generated from the RackHD code. Manually editing this will lead to future issues.

@kacole2 kacole2 mentioned this pull request May 23, 2016
@kacole2
Copy link
Contributor

kacole2 commented May 23, 2016

If you can get RackHD/on-http#292 merged, I won't hesitate to move this further. But I think that monorail.yml is our "source of truth"

@codenrhoden
Copy link
Collaborator Author

I totally agree about the upstream monrail.yml being our source of truth. I'd like them to be identical soon.

I also apologize - I totally missed your PR, RackHD/on-http#175, when I submitted all of mine to RackHD. You had already solved several of the problems I hit. :(

@kacole2
Copy link
Contributor

kacole2 commented May 26, 2016

@codenrhoden same here as #17. anything that needs to be updated before merge since it has been included upstream?

Many of the return schemas here are actually arrays of objects,
not just a single object. Makes for more accurate generated code
from Swagger.
@codenrhoden
Copy link
Collaborator Author

@kacole2 this is now good to go as well. Passed the 4 local tests for GET/POST/DELETE/Lookup...

I may do one more PR in the the next few days to re-synch the two monorail.yml files for good, but right now things are looking great! Thanks for all your help.

@kacole2 kacole2 merged commit 3eadf9e into master May 27, 2016
@codenrhoden codenrhoden deleted the return_arrays branch June 27, 2016 16:26
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.

None yet

2 participants