-
Notifications
You must be signed in to change notification settings - Fork 34
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
Enable polymorphism #30
Conversation
…rties object and a single requirements array.
@Kitanotori Thanks for the contributions here. Are there any unit tests that you can add to ensure that this functionality doesn't undergo regression in the future? |
If not, I plan to go back and add some tests for the |
Unfortunately no time to write tests currently. |
This does not handle
|
This is what I came up with to solve the problem I mentioned above: celalo@8c4370a |
… it), add mistakenly removed mongoose dependency back to devDependencies
…cessary and add necessary peer dep
# Conflicts: # package.json
I just saw this! Thanks for pushing forward on these updates! From what I can tell the Travis job is failing due to some nested dependencies that no longer support node 0.11. Feel free to update to 4+ in the travis.yml and see if we can get the tests to pass here. |
… into process-allof
… into process-allof
@briananderson1222 I fixed Travis's and Mocha's settings (timeout limit was too low). Now only the test I added, containing example of allOf definition, fails. |
… it), add mistakenly removed mongoose dependency back to devDependencies
- date and date-time were not processed correctly when their type was correctly specified as ‘string’ - date, dateTime, password, double, float, long are not valid types
# Conflicts: # package.json # test/index.js
…g’s except ‘date’ and ‘date-time’ as String
if (!_.isEmpty(mergedReqs)) definition['required'] = Array.from(mergedReqs); | ||
delete definition['allOf']; | ||
} | ||
|
||
var getSchema = function (objectName, fullObject) { | ||
var props = {}; | ||
var required = fullObject.required || []; |
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.
Line 247 (var required = fullObject.required || [];
) should be moved to line 251 (i.e. after processPolymorphic runs if the definition is polymorphic) otherwise we're not adding in the merged required fields.
…dency to ^5.0.0
...by merging members of "allOf" into a single properties object and a single requirements array.
Fixes #29