Conversation
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
What is purpose of using eslint?
It would be better to define pre-commit hook that will run eslint for avoiding non-codestyle code in repo.
Take a look pre-commit lib, please.
If you would like to implement it, just do it 😄
If not just skip this comment.
FYI: Using eslint is out of scope homework task and will no be estimate.
There was a problem hiding this comment.
I've added =)
Not sure that I've done everything right, but anyway looks like it works!
README.md
Outdated
| @@ -1 +1,4 @@ | |||
| # node-js-course | |||
|
|
|||
| ## npm install | |||
There was a problem hiding this comment.
All code should be into code blocks.
Use code blocks, please
code-blocks
FYI: # using via header. headers
package.json
Outdated
| "name": "node-js-course", | ||
| "version": "1.0.0", | ||
| "description": "", | ||
| "main": "index.js", |
There was a problem hiding this comment.
Rename to application entry point, please
FYI: ./dist/app.js
package.json
Outdated
| }, | ||
| "homepage": "https://github.com/sasha-klimashevich/node-js-course#readme", | ||
| "dependencies": { | ||
| "babel-cli": "^6.26.0", |
There was a problem hiding this comment.
Remove all ^, please
FYI: The npm creates package-lock.json. But if user uses npm less than 5 version. The ^ effects to download up-to-date from resource. It might be problem for running application.
using-a-package
package.json
Outdated
| "dependencies": { | ||
| "babel-cli": "^6.26.0", | ||
| "babel-core": "^6.26.0", | ||
| "babel-preset-env": "^1.6.1", |
There was a problem hiding this comment.
Babel's module should not be in dependencies section. They should be in devDep.
Check what modules should in devDep section, please
src/app.js
Outdated
|
|
||
| console.log(data.name); | ||
|
|
||
| new models.User(); |
There was a problem hiding this comment.
import { User, Product } from './models';
Why not?
In this way we can make influence what a bundle will contain.
There was a problem hiding this comment.
Sounds good.
Done!
package.json
Outdated
| "scripts": { | ||
| "prestart": "babel src -d dist", | ||
| "test": "echo \"Error: no test specified\" && exit 1", | ||
| "start": "nodemon ./dist/app.js" |
There was a problem hiding this comment.
Use ./node_modules/.bin/nodemon ./dist/app.js, please
FYI: If this case i need install nodemon globally, but if i don't wish install it globally. A case above provides us to avoid this issue.
There was a problem hiding this comment.
Done, thx
Well.. I don't have nodemon installed globally and this command worked for me. What's the difference ?
| "description": "", | ||
| "main": "index.js", | ||
| "scripts": { | ||
| "prestart": "babel src -d dist", |
There was a problem hiding this comment.
Use prestart, using nodemon doen't make sence. (will be discussed fbf)
|
When comments will be fixed write |
2 participants
@vladislavkovaliov