Use prettier to format the code

[bajtos]

• Move prettier config from VSCode settings to a standalone file
• Add npm run prettier:check for verifying formatting
• Add npm run prettier:fix to automatically reformat the code
• Integrate prettier check into npm test pipeline
• Enable "format on save" in VS Code settings (yay 🕺)
• Reformat all existing code

[bajtos]

A note for reviewers: the first commit (4c8f72e) contains changes in project infrastructure, the second commit (d58127b) contains formatting changes in TypeScript sources. I hope this will make it easier for you to review this patch.

I'll squash both commits into a single one before merging.

[virkt25]

LGTM. Just a comment regarding checking this in our npm test workflow. I think it might be better to have different stage for linting on travis where we run npm run lint && npm run prettier:check. I just find it easier to look at Travis and know where something went wrong without having to open up logs.

Something like the following job in .travis.yml

- stage: Linting
      node_js:
        - "8"
      before_script: skip
      script: npm run lint && npm run prettier:check
      after_script: skip

[bajtos]

LGTM. Just a comment regarding checking this in our npm test workflow. I think it might be better to have different stage for linting on travis where we run npm run lint && npm run prettier:check. I just find it easier to look at Travis and know where something went wrong without having to open up logs.

Makes sense, it would be good improvement. Let's leave it out of scope of this pull request though, as it's a different change IMO. Created a GH issue to keep track of this - see #610.

This actually raises a question - I think npm run lint should execute both tslint and prettier, because both tools are checking the coding style. Thoughts?

[bajtos]

I think npm run lint should execute both tslint and prettier, because both tools are checking the coding style. Thoughts?

Pushed a new commit a641331 to make this happen. npm run lint will run both linters now, npm run lint:fix will run both checkers in the auto-fix mode.

[bajtos]

@virkt25 PTAL again.

@kjdelisle @raymondfeng do you have any opinion on this change? May I go ahead and land this patch?

[virkt25]

Makes sense, it would be good improvement. Let's leave it out of scope of this pull request though, as it's a different change IMO. Created a GH issue to keep track of this - see #610.

+1

I think npm run lint should execute both tslint and prettier, because both tools are checking the coding style.

+1

[raymondfeng]

What rule control this? Isn't ctx.bind('spot').to(new Dog()).tag('dog'); perfectly readable?

[bajtos]

Looks like this is hard-coded, at least I don't see any option to control this: https://github.com/prettier/prettier/blob/master/docs/options.md

[virkt25]

//prettier-ignore can be used to skip lines / blocks we want to be skipped by prettier.

Just an option. I do think we should keep prettier formatting wherever possible to keep things consistent.

(Also if the result of a chain is being assigned to variable then it doesn’t break it down to a command per line)

[raymondfeng]

Interesting, I don't know that we can use in keyword as object key without quotation.

[raymondfeng]

LGTM with a few minor questions/comments.