-
-
Notifications
You must be signed in to change notification settings - Fork 126
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
Update TypeScript to 3.8.3 and Prettier to 2.0.4 #248
Conversation
@xaviergonz gonna tag you because you were active here recently 😄 |
Please do make sure you don't commit unrelated files. Your prettier configs seems to deviate from what is used in this repo, making this PR much larger than it should be. Feel free to commit a |
@@ -8,7 +8,7 @@ const readline = require("readline") | |||
|
|||
const rl = readline.createInterface({ | |||
input: process.stdin, | |||
output: process.stdout | |||
output: process.stdout, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be the result of using a different prettier config or version. Please align your local settings with this repo, or don't stage the unrelated files, or creating a PR that locks the config and prettier version would be even more awsome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No unrelated files touched by this PR
Yeah sorry, this PR should be called "update typescript and prettier". Prettier 2 supports the TS 3.8 features. Prettier 2 contains some breaking changes concerning default options and formatting, hence the number of changes. See also https://prettier.io/blog/2020/03/21/2.0.0.html As the new defaults make sense I thought I'd just reformat everything although I guess we could manually change the config so it doesn't change much. |
@NaridaL that makes sense, thanks for the explanation! The new dangling commas everywhere, is that a change in the prettier defaults? If it isn't, the .prettierrc should be updated accordingly |
Answer: yes, it is the new default |
@mweststrate do you mind hitting merge? Or you can add me as maintainer here too. Also, I'd appreciate you taking a look at #245 😄 |
@NaridaL sorry, very swamped atm! I'll send an invite. |
I want to use ?. in my other pull request.