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

[reviews] list of potential issues #25

Open
nartc opened this issue Jul 7, 2019 · 0 comments
Open

[reviews] list of potential issues #25

nartc opened this issue Jul 7, 2019 · 0 comments

Comments

@nartc
Copy link

nartc commented Jul 7, 2019

  1. Do not expose connectionString
    https://github.com/quangpl/cnpm-ezlife/blob/master/config.js#L3

  2. Try to use const for imports, and bring all imports to the top of the file.
    https://github.com/quangpl/cnpm-ezlife/blob/master/app.js#L8

  3. Try to use ref. The current usage is fine but if you go with storing the IDs of related models, try to use ref from mongoose.

Consider extracting this schemaOptions object out to a const:

{
    timestamps: true,
    versionKey: false
}

https://github.com/quangpl/cnpm-ezlife/blob/master/schemas/amount_report.js#L7

  1. Just do return res.render(...) instead
    https://github.com/quangpl/cnpm-ezlife/blob/master/routes/page.js#L18

  2. When you use async/await, if you don't have try/catch to handle errors, you don't need to return await ...

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

No branches or pull requests

1 participant