Conversation
There was a problem hiding this comment.
Do we really need to add a rule to complain about not being explicit everywhere? A lot of these typings are obvious based on the code (which is probably why they were left out), and VS Code IntelliSense already lets you know the type when the appropriate typings are found. I find this rule to be a bit too cumbersome.
There was a problem hiding this comment.
(BTW, I meant to put this comment on line 87 which makes a little more sense than putting it here - sorry!)
There was a problem hiding this comment.
Are always using explicit types recommended typescript practice? I assume you could omit the type when the compiler is able to determine it, like with "auto" in C++. I think making the type explicit is good for readability when it's unclear, but often times it doesn't really help and just adds extra text.
There was a problem hiding this comment.
Yes, explicit types is recommended Typescript practice. If the type is a dictionary, feel free to use any. However most of the time, the type should be defined.
On the rare chance that this is too much (e.g. https://github.com/WardenGnaw/vscode-cpptools/blob/5c231a544c283a0ef6fb053e703846eff76e350f/Extension/src/LanguageServer/protocolFilter.ts#L13) then disabling the linter is fine.
Extension/src/main.ts
Outdated
There was a problem hiding this comment.
is there a rule to enforce correct formatting of if-else blocks? The "else" should be on the line above next to the {
Adding tslint and the associated changes that it complains about. Adding gulp to do tests, tslint, and more. Including package-lock.json that it keeps complaining about.
Added npm run test, watch, and tslint.
new-parens for forcing all constructor calls to have parenthesis. no more than 1 consecutive blank lines no useage of String(), Boolean(), or Number(), use native type.
WardenGnaw
force-pushed
the
dev/waan/lintandgulp
branch
from
5bbb94e to
5c231a5
Compare
|
|
||
| const lintReporter = (output, file, options) => { | ||
| //emits: src/helloWorld.c:5:3: warning: implicit declaration of function ‘prinft’ | ||
| var relativeBase = file.base.substring(file.cwd.length + 1).replace('\\', '/'); |
There was a problem hiding this comment.
see if you could get source navigation from the lint output to code, from vscode build output.
| var handleDownloadFailure = (num, error) => { | ||
| let lastError: any = null; | ||
| let retryCount: number = 0; | ||
| let handleDownloadFailure: (num: any, error: any) => void = (num, error) => { |
| return this.DownloadFile(pkg.url, pkg, 4).catch((error) => { handleDownloadFailure(5, error); | ||
| return this.DownloadFile(pkg.url, pkg, 5); // Last try, don't catch the error. | ||
| })})})})}).then(() => { | ||
| return this.DownloadFile(pkg.url, pkg, 0).catch((error) => { |
|
👍 |
4 participants
Adding tslint and the associated fixes that it complains about.
Adding gulp to do tests, tslint, and more.
Including package-lock.json that it keeps complaining about.