-
Notifications
You must be signed in to change notification settings - Fork 513
Overhaul linting, formatting, and testing infrastructure #5190
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
Conversation
58f1dd0
to
c82afed
Compare
c82afed
to
fccf1f6
Compare
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.
Pull Request Overview
This PR completely overhauls our linting, formatting, and testing infrastructure. It replaces legacy import and configuration patterns with modern TypeScript type imports and updated configuration files, while also revising workspace settings and dependency versions.
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/logging.ts | Modernized logger initialization using nullish assignment and updated imports |
src/features/* | Updated type imports and removed eslint-disable comments for empty interfaces |
src/extension.ts | Adjusted import types and refined inline comments |
pwsh-extension-dev.code-workspace | Revised workspace settings with updated file patterns and new extension recommendations |
package.json, eslint.config.mjs, etc. | Updated dependency versions and overhauled configuration to support new standards |
Comments suppressed due to low confidence (2)
src/logging.ts:143
- The use of the nullish assignment operator (??=) is concise; ensure that the target environment and TypeScript version fully support this syntax.
this._channel ??= window.createOutputChannel(this.channelName, {log: true});
pwsh-extension-dev.code-workspace:314
- The new 'outFiles' pattern is more restrictive; make sure it matches all emitted JavaScript files required for proper source map support.
"${workspaceFolder:Client}/dist/*.js"
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.
LGTM!
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.
LGTM! Nice work on the test fixes. Just some future notes.
"@vscode/vsce": "^3.3.0", | ||
"esbuild": "^0.25.1" | ||
"@vscode/vsce": "^3.3.2", | ||
"esbuild": "^0.25.4" | ||
}, | ||
"optionalDependencies": { |
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.
We probably should document somewhere why optionalDependencies vs devdependencies
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.
I'm really hoping it changes when I get back to trying #5063
Replaces #5188 with a whole overhaul of our linting, formatting, and testing infrastructure.