Change error message displayed when path to a tool is invalid #1903
Conversation
|
|
||
| 'use strict'; | ||
|
|
||
| // tslint:disable:max-classes-per-file |
There was a problem hiding this comment.
Would it be so bad to split this into multiple files representing each implementation? (Not a dealbreaker of course, but perhaps a subdir with each implementation might be more understandable).
There was a problem hiding this comment.
I would've done that, but this is how it was written by Mikhail, hence I left it.
Generally, I'd split it, however if the classes are small, I don't mind having them in one file either.
| @@ -50,6 +50,14 @@ export enum InstallerResponse { | |||
| Ignore | |||
| } | |||
|
|
|||
| export enum ProductType { | |||
|
|
||
| 'use strict'; | ||
|
|
||
| // tslint:disable:max-func-body-length no-invalid-this |
There was a problem hiding this comment.
Is this necessary or could we split the case-statement bodies into callable methods that would shorten the body length (and perhaps improve readability?).
There was a problem hiding this comment.
Was trying to avoid some tests getting skipped. The down side is that we will have a large number of tests (144 to be precise) that get executed but are skipped.
Here's what we get:
The only way to avoid this is to write a case statement or split each case statement into a separate test suite, which is a lot more work/code.
This is what it looks like today (with the case statement):
Codecov Report
@@ Coverage Diff @@
## master #1903 +/- ##
=========================================
- Coverage 74.46% 74.4% -0.07%
=========================================
Files 283 285 +2
Lines 13287 13356 +69
Branches 2387 2390 +3
=========================================
+ Hits 9894 9937 +43
- Misses 3259 3280 +21
- Partials 134 139 +5
Continue to review full report at Codecov.
|
DonJayamanne
force-pushed
the
issue1064LinterPath
branch
from
2dc3b89 to
d4cde31
Compare
Fixes #1064
This pull request: