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

Enhancement/populate method - #137 #250

Merged
merged 3 commits into from Jan 2, 2018
Merged

Enhancement/populate method - #137 #250

merged 3 commits into from Jan 2, 2018

Conversation

lucianopf
Copy link
Contributor

@lucianopf lucianopf commented Dec 4, 2017

PR to finish the development of #137. (Adding support for populate method)

1st of all I'm sorry for pushing this all from this branch, but I felt we need to separate test schemas from the actual test file.

So, by that I've moved the schemas to fixtures/Cats.js and did change all the references to use this new source of it.

This PR implements:

1 - New fixtures folder structure.
2 - The support for ref param at a schema attribute.
3 - New usage of cat.populate({ path: 'parent' }) or cat.populate('parent').
4 - A few tests to cover that new implementation.

That's some sort of initial proposal with obvious enhancements to be done for example at the validation and the tests.

But I feel like that's enough to be added to the project so other devs can help us to enhance that feature. =)

@lucianopf lucianopf changed the title Enhancement/populate method Enhancement/populate method - #137 Dec 4, 2017
@coveralls
Copy link

coveralls commented Dec 4, 2017

Coverage Status

Coverage increased (+0.3%) to 79.032% when pulling f03e918 on lucianopf:enhancement/populate-method into fdf8a7c on automategreen:master.

@lucianopf
Copy link
Contributor Author

@brandongoode that's just waiting to be reviewed =D
I'll address any needed changes =D

@brandongoode
Copy link
Contributor

@lucianopf -- looks great - sorry for the long delay. I took some time off for Christmas and the flu.

@brandongoode brandongoode merged commit 0ac58cf into dynamoose:master Jan 2, 2018
@lucianopf
Copy link
Contributor Author

No problem buddy! I hope you're feeling better @brandongoode! ^^

Are there any other changes needed at Dynamoose? 🤔

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

3 participants