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

Populate the development database with dummy users #312

Merged
merged 14 commits into from
Oct 3, 2020

Conversation

Be-Like
Copy link
Contributor

@Be-Like Be-Like commented Oct 1, 2020

Refer to #279.

@AlbertoPdRF So far I have only created a script to populate the Users and Tokens associated with those users. Before I went too much further with the other tables, I wanted to get your feedback on the current progress I've made. I added a dev dependency that will auto generate the names for the users.

Please let me know if there are any changes you would like me to make. I plan on creating the other scripts similarly.

I also plan on creating a parent script that will run all the other scripts with one command and to simplify the code.

@Be-Like Be-Like marked this pull request as draft October 1, 2020 18:27
@AlbertoPdRF AlbertoPdRF added back end Requires work on the back end of the application dependencies Related to external dependencies enhancement Related to improvements of existing features and/or addition of new ones labels Oct 1, 2020
Copy link
Owner

@AlbertoPdRF AlbertoPdRF left a comment

Choose a reason for hiding this comment

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

Wow, thanks for this amazing first contribution @Be-Like! This will for sure be very much appreaciated by any future contributor! 🤩

I've left a bunch of comments for you (sorry if they are too many! 🙏)!

Thanks again for such an amazing

Besides, to follow the convention we are using regarding file names, the new script should be named using - instead of _, and it would be better to rename the interests script to populate-interests.js instead of just populate.js.

Thanks again for contributing! I'll assign #279 to you 🙂

EDIT: oops, it seems I can't assign the issue to you, at least not until you leave a comment on it.

api/db/populate_users.js Outdated Show resolved Hide resolved
api/db/populate_users.js Outdated Show resolved Hide resolved
api/db/populate_users.js Outdated Show resolved Hide resolved
api/db/populate_users.js Outdated Show resolved Hide resolved
api/db/populate_users.js Outdated Show resolved Hide resolved
api/db/populate_users.js Outdated Show resolved Hide resolved
api/db/populate_users.js Outdated Show resolved Hide resolved
api/db/populate_users.js Outdated Show resolved Hide resolved
api/db/populate_users.js Outdated Show resolved Hide resolved
api/db/populate_users.js Outdated Show resolved Hide resolved
@AlbertoPdRF AlbertoPdRF changed the title 279 dummy data Populate the development database with dummy users Oct 1, 2020
@Be-Like
Copy link
Contributor Author

Be-Like commented Oct 2, 2020

Thanks for the feedback! I'll take a look at your comments and let you know if I have any questions. I also added a comment to #279 so it can be assigned.

@AlbertoPdRF
Copy link
Owner

Wonderful! 💯

By the way, let's not forget to keep the API's README updated with the changes that you are doing. A good addition after all of the work is done would be to add an npm script named populate-db or something similar that takes care of populating the database running the corresponding scripts. This way, the user doesn't need to run the command listed on the API's README, and we could also move that new instruction to the main one!

@Be-Like
Copy link
Contributor Author

Be-Like commented Oct 2, 2020

So we can get this PR done more quickly (especially for any others that may be joining for hacktoberfest) and to avoid adding too much complexity to this PR, I think we should open some other feature tickets to expand on this one.

  • Adding command line flags and options
  • Simplifying what is logged
  • Image generator

@AlbertoPdRF
Copy link
Owner

That sounds good to me @Be-Like!

@AlbertoPdRF
Copy link
Owner

Ok, this looks quite good to me! Let's resolve the remaining Prettier issues and get it merged 🚀 Then we can go with all the follow up issues 😄

Copy link
Owner

@AlbertoPdRF AlbertoPdRF left a comment

Choose a reason for hiding this comment

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

Just two small comments, sorry I didn't see this earlier!

api/db/populate-users.js Outdated Show resolved Hide resolved
api/db/populate-users.js Outdated Show resolved Hide resolved
Copy link
Owner

@AlbertoPdRF AlbertoPdRF left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks so much for this awesome first contribution @Be-Like! 📈

@Be-Like Be-Like marked this pull request as ready for review October 3, 2020 19:41
@AlbertoPdRF AlbertoPdRF merged commit 135964a into AlbertoPdRF:master Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back end Requires work on the back end of the application dependencies Related to external dependencies enhancement Related to improvements of existing features and/or addition of new ones
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants