modernize tooling and resulting starter package #109
Conversation
Use ESLint instead of TSLint; upgrade tsconfig.json.
Use ESLint instead of TSLint, use yarn instead of npm, use newer JSON Schemas for validation.
|
🎉 This PR is included in version 1.8.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
felixfbecker
left a comment
felixfbecker
left a comment
There was a problem hiding this comment.
Thank you for the PR! It is much appreciated, but please wait for code owner approval next time before merging :)
| }, | ||
| } | ||
| await writeFile('tslint.json', JSON.stringify(tslintJson, null, 2)) | ||
| await writeFile('eslint.json', JSON.stringify(eslintJson, null, 2)) |
There was a problem hiding this comment.
This should be .eslintrc.json
|
|
||
| console.log('📄 Adding .gitignore') | ||
| await writeFile('.gitignore', ['dist/', 'node_modules/', '.cache/', ''].join('\n')) | ||
| await writeFile('.gitignore', ['dist/', 'node_modules/', '.cache/', 'yarn.lock', ''].join('\n')) |
There was a problem hiding this comment.
yarn.lock shouldn't be gitignored
| console.log('📦 Installing dependencies') | ||
| await exec( | ||
| 'npm', | ||
| 'yarn', |
There was a problem hiding this comment.
We should definitely use yarn in our repo for consistency, but should we really force extension authors to use yarn? I'm worried that some might not even have it installed (especially if they're not from the JS ecosystem) and then it becomes one more thing they need to figure out they have to install manually in addition to just NodeJS. And then if you go to https://yarnpkg.com, it shows you the documentation for Yarn v2, but we're relying on Yarn v1 here and Yarn v2 will not work. All this could be very confusing for a user.
With this concern, the question for me becomes "what is the incremental value extension authors get out of yarn over npm?" (and does it outweigh the concern), and I don't see any significant value. They have few dependencies, so performance difference is not noticeable, and latest npm is almost as fast as yarn (and no longer has the annoying lockfile bugs that made us switch originally).
|
|
||
| if (await exists('tslint.json')) { | ||
| console.log('📄 tslint.json already exists, skipping creation') | ||
| if (await exists('eslint.json')) { |
There was a problem hiding this comment.
This should be .eslintrc.json
See commit messages.