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

Samples for swagger-js/#417 #384

Merged
merged 8 commits into from
Jun 5, 2015
Merged

Samples for swagger-js/#417 #384

merged 8 commits into from
Jun 5, 2015

Conversation

jharmn
Copy link
Contributor

@jharmn jharmn commented Jun 1, 2015

YAML only for now, to start the discussion. Once we have a base set of examples, I can generate JSON and update the spec with these samples.

@webron
Copy link
Member

webron commented Jun 1, 2015

@jasonh-n-austin - couple of thoughts before merging:

  • Since you refer to petstore-separate-common.yaml (for the ../... ref), maybe all should be moved into another level of a sub-directory.
  • Unfortunately, the YAML examples are not too good, and need to be modified. Would be great if you could fix it already in your generated example:
    • All model definitions should have type: object in them.
    • The composition sample is bad because the NewPet redefines the same property as adjacent Pet. You don't have to change this one, but it would be nice.
  • The tests are failing :) Can't merge it without a fix to those.

@jharmn
Copy link
Contributor Author

jharmn commented Jun 1, 2015

I was thinking a sub-dir was a better idea too.
I assumed this wasn't in mergeable state yet, wanted to get something started to give @fehguy some test data for swagger-js/#417.

Great feedback, I'll shape things up this afternoon.

@jharmn jharmn changed the title Samples for #417: Separated files, yaml only for now Samples for swagger-js/#417: Separated files, yaml only for now Jun 1, 2015
@jharmn
Copy link
Contributor Author

jharmn commented Jun 5, 2015

@webron moved separate files into their own directory. For now, added exclusion to YAML section of validate.js to skip tests for schema files in petstore-separate...not really positive how to best run schema validator against those (notably because parameters.yaml is a swagger fragment, and the schema files are really JSON Schema). Now tests are at least validating that petstore-separate/spec/swagger.json

@jharmn
Copy link
Contributor Author

jharmn commented Jun 5, 2015

Will look at NewPet/Pet/oneOf fix now.

@webron
Copy link
Member

webron commented Jun 5, 2015

Ok, thanks.

@jharmn
Copy link
Contributor Author

jharmn commented Jun 5, 2015

@webron Just pushed fixed example for petstore-expanded(.json&&.yaml). Now NewPet is the base, extended by Pet (adds id).

@webron
Copy link
Member

webron commented Jun 5, 2015

Looks great, thanks! Could I just trouble you to add "type": "object" to Pet, NewPet and Error in the json variant?

@jharmn
Copy link
Contributor Author

jharmn commented Jun 5, 2015

@webron OK added type: object to existing JSON examples.
Also:

  • Generated JSON versions of petstore-separate
  • Test fixes for json/petstore-separate exclusions

@webron
Copy link
Member

webron commented Jun 5, 2015

Great, thanks for all the work.

webron added a commit that referenced this pull request Jun 5, 2015
Samples for swagger-js/#417: Separated files, yaml only for now
@webron webron merged commit 7926841 into OAI:master Jun 5, 2015
@jharmn jharmn changed the title Samples for swagger-js/#417: Separated files, yaml only for now Samples for swagger-js/#417 Jun 5, 2015
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