Fix whole of style (Extends #361) #474
Conversation
|
Could someone review this? |
| @@ -20,7 +20,7 @@ suite('Extension Tests', () => { | |||
| y | |||
| } | |||
| `.split('\n'); | |||
| function f(i) {return (doc[i]); } | |||
| function f(i: number) { return (doc[i]); } | |||
There was a problem hiding this comment.
Here on my side, eslint complains about the use of the deprecated assert.equal:
Should we do anything about it?
There was a problem hiding this comment.
Thank you for your point.
Maybe it is not eslint, it is a typescript function alert.
But It's good timing, I'll fix it to assert.strictEqual
|
I go through the changes and all look good. Tested on macOS and everything works well. |
| import * as vscode from 'vscode'; | ||
| import { commands, window, QuickPickItem, Uri, Webview, WebviewPanel, WebviewOptions, WebviewPanelOnDidChangeViewStateEvent, ViewColumn } from 'vscode'; |
There was a problem hiding this comment.
Is there a performance benefit from importing each item explicitly or is this just a stylistic choice?
There was a problem hiding this comment.
It is not alerted by ESLint and performance issue, but it is useful to refactoring based on the vscode API.
In this case, we can confirm rHelpPanel.ts manage the Webview API related function.
If someone implements a related function in another file, we should combine that in this file or implement super-class
|
I merge this, thank you @renkun-ken and @ManuelHentschel for your comments. |
What problem did you solve?
#361 was too large to solve all issues.
This PR only apply the easy fix, and I gave up that fixing all rule violation once, especially for the
anyvalue warnings.I want to fix remained violations in another issue, after merge this.
I confirmed that fixing current new functions source code readability is very hard work.
This PR can be the first step to keeping this extension maintainability
Next TODO:
Refactor any values it can be object
Simplify extension.ts