Repo setup #6
Repo setup #6
Conversation
michielbdejong
force-pushed
the
repo-setup
branch
from
0bde3c7 to
b2c9b10
Compare
michielbdejong
force-pushed
the
repo-setup
branch
from
f5a5ea6 to
f3ef0fa
Compare
src/server.ts
Outdated
| import Debug from 'debug' | ||
|
|
||
| const debug = Debug('server') | ||
| const port = 8080 |
There was a problem hiding this comment.
const port = process.env.PORT || 8080
There was a problem hiding this comment.
Or better yet: make Server a class so it is testable; and 8080 is a default for its parameters. Classes should not depend on process variables, that's up to the script wiring up things.
src/server.ts
Outdated
| import Debug from 'debug' | ||
|
|
||
| const debug = Debug('server') | ||
| const port = 8080 |
There was a problem hiding this comment.
Or better yet: make Server a class so it is testable; and 8080 is a default for its parameters. Classes should not depend on process variables, that's up to the script wiring up things.
tsconfig.json
Outdated
| { | ||
| "env": "node", | ||
| "compilerOptions": { | ||
| "target": "es6", |
There was a problem hiding this comment.
Use es2015 rather than es6 (that name was abandoned a long while ago)
tsconfig.json
Outdated
| "target": "es6", | ||
| "module": "commonjs", | ||
| "moduleResolution": "node", | ||
| "noImplicitAny": false, |
There was a problem hiding this comment.
I'd recommend to keep the TypeScript compiler as strict as possible from the start of the project. I'll be easier that way. Making it stricter later on will prove difficult.
Most of the time, it isn't that hard to be explicit about types and to limit the use of "any"
| "debug": "^4.1.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/chai": "^4.1.7", |
There was a problem hiding this comment.
I don't know how you feel about this, but I'd advise against using ^ or ~ for version ranges. I love semver, but third-party developers can rarely be trusted to respect it systematically. For stability, keeping stricter control is usually nicer and avoid surprises.
There was a problem hiding this comment.
isn't that handled by package-lock.json? When I run npm install, it will look for the latest versions of everything, and update the package-lock.json. Then I run my tests and if everything still works, commit the new package-lock to git and to npm. Then if someone installs my package through npm, they get the versions that are in the package-lock. That way every user gets the same dependency versions as the ones I tested with. Right?
There was a problem hiding this comment.
Indeed, but what happens is that you'll get patch updates that create problems that your test suite doesn't show you; you lose control in that sense.
Sometimes there are many changes in the package-lock.json and it's impossible to see the ramifications.
When you keep stricter control over dependencies, then you know exactly what you've just changed and things become more tractable.
https://renovatebot.com/docs/dependency-pinning/
What I personally like to do it deploy something like greenkeeper (https://greenkeeper.io/) to automatically propose version upgrades as pull requests. Here's an example: NationalBankBelgium/stark#1206
That way things remain under control and it eliminates surprises while remaining manageable
| @@ -0,0 +1,10 @@ | |||
| { | |||
| "extends": ["tslint-config-standard"], | |||
There was a problem hiding this comment.
Don't know if you're aware, but TSLint will be abandoned in favor of ESLint by its creators (Palantir): https://medium.com/palantir/tslint-in-2019-1a144c2317a9
There was a problem hiding this comment.
I didn't know! They say "Once we consider ESLint feature-complete w.r.t. TSLint, we will deprecate TSLint and help users migrate to ESLint" so we'll stay on TSLint until they tell us it's time to switch.
package.json
Outdated
| "Michiel de Jong (https://github.com/michielbdejong)", | ||
| "Ruben Verborgh (https://github.com/rubenverborgh)", | ||
| "Kjetil Kjernsmo (https://github.com/kjetilk)", | ||
| "*Jackson Morgan (https://github.com/jaxoncreed)", |
package.json
Outdated
| "Pat McBennett (https://github.com/pmcb55)", | ||
| "Justin Bingham (https://github.com/justinwb)" | ||
| ], | ||
| "license": "ISC", |
There was a problem hiding this comment.
Should be MIT?
|
Remark from @kjetilk : don't hard-code port in test |
|
Thanks all! |
No description provided.