Uh oh!
There was an error while loading. Please reload this page.
Initialize sequelize - #120
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
allella
commented
Oct 21, 2019
timmyichen
left a comment
There was a problem hiding this comment.
Nice start! Had a few comments about the docker setup.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
vaibhavsingh97
commented
Oct 24, 2019
@Zeko369 Let me know if you stuck somewhere. We can help you, just ping me on discord or any mods. Also, please resolve merge conflict |
QuincyLarson
commented
Oct 28, 2019
@Zeko369 Thanks again for pushing forward with the sequelize / sequelize-ts integration. Have you had a chance to make the suggested changes and finish your checklist? This is now critical path. Please let us know if we can do anything to help. |
vaibhavsingh97
commented
Oct 28, 2019
@Zeko369 can you please give me push access so that I can help you out, and help you complete the checklist |
timmyichen
commented
Oct 28, 2019
Alternatively since this PR is good on its own it could be merged and follow up PRs could be made |
| "test": { | ||
| "username": "postgres", | ||
| "password": "password", | ||
| "database": "chapter", |
There was a problem hiding this comment.
We should use a different db for testing, one that we bring up and tear down when testing. Not essential for this pr, just leaving a note
timmyichen
left a comment
There was a problem hiding this comment.
Hooray, thanks! :) One change I would recommend would be to change all table and column names to snake_case (including the fields on the model class) as that's a postgresql best practice. We can use the camelize package to camelize fields before returning them from the API.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Hi, how can I help out? My skills are mainly on DB and backend of systems. |
timmyichen
commented
Oct 29, 2019
On second thought @Zeko369 any particular reason for the pattern of having |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| "@types/morgan": "^1.7.37", | ||
| "@types/next": "^8.0.6", | ||
| "@types/node": "^12.11.1", | ||
| "@types/node": "^12.11.2", |
There was a problem hiding this comment.
Isn't Node 10 being used currently?
Line 4 in c1cf448
allella
commented
Nov 6, 2019
@timmyichen it looks like @vaibhavsingh97 is still showing as requesting changes. @vaibhavsingh97 can you review and approve if this is ready. Also, the package-lock.json is showing in conflict. |
Zeko369
commented
Nov 6, 2019
@allella@timmyichen I completely forgot about this PR, I fixes most of the requests, I only have one question. What are we going to do about the seeds?, default |
@nik-john I dont know how you managed to get the CI pass for #158 , because I just merger master and you had a TS bug that stops it from compiling it on the CI. <StyledComponentsThemeProvidertheme={themeObject}><Fragment><CssBaseline/><Component{...pageProps}/></Fragment></StyledComponentsThemeProvider>you missed the fragment :) |
vaibhavsingh97
commented
Nov 6, 2019
@Zeko369 what are your thoughts? IMO, we should intitalize the database connection and export the connection to app, so that we can reuse it to query the database. |
nik-john
commented
Nov 16, 2019
@all-contributors please add @Zeko369 for code tool |
I've put up a pull request to add @Zeko369! 🎉 |
Uh oh!
There was an error while loading. Please reload this page.