-
Notifications
You must be signed in to change notification settings - Fork 110
Bump eslint/typescript and update Typescript #450
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
base: main
Are you sure you want to change the base?
Conversation
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.
PR Overview
This PR upgrades Typescript, eslint, and typescript-eslint to the latest versions and transitions to an mjs style configuration to address linting errors.
- Introduces a new mjs configuration file for eslint.
- Ensures the configuration aggregates recommended settings from eslint and typescript-eslint.
Reviewed Changes
File | Description |
---|---|
eslint.config.mjs | Adds a new mjs configuration file combining recommended settings from eslint and typescript-eslint. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
@AnthonyBorton please review, thanks :) |
@JustinGrote thanks for this! Could you look into the build failures? |
Newer eslint has diff options, I'll update the pipeline |
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 updates dependency versions and configuration for Typescript, ESLint, and typescript-eslint while switching to an mjs style configuration to address linting errors in upcoming PRs.
- Added a new ESLint configuration file (eslint.config.mjs) that integrates recommended settings from ESLint and typescript-eslint.
- Updated the build workflow to use Node version 20.x.
- Adjusted icon utility functions including changes to return types in icons.ts and minor refactorings in tree view nodes.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
eslint.config.mjs | New mjs configuration file integrating ESLint and typescript-eslint recommended configs. |
.github/workflows/build.yml | Updated Node version to 20.x for the build process. |
src/treeViews/icons.ts | Updated icon helper functions; changed return type of getIconForWorkflowStep. |
src/treeViews/shared/workflowJobNode.ts | Refactored constructor formatting for clarity. |
src/treeViews/shared/workflowRunNode.ts | Removed an extraneous newline to improve code clarity. |
Comments suppressed due to low confidence (1)
src/treeViews/icons.ts:87
- The change from returning an empty string to undefined in getIconForWorkflowStep may impact downstream consumers expecting a string. Verify that all usages properly handle an undefined value.
return undefined;
@joshmgross fixed up, please re-enable the workflow to test. It passed in my Actions: https://github.com/JustinGrote/vscode-github-actions-enhanced/actions/runs/13800804697/job/38602688747 b2c15d8 is a summary commit of the changes since last review |
b986529
to
805146b
Compare
- Bump latest esbuild version explicity - Set CI Node to VSCode 1.98.0 Node Version - Fixup vscode tooling and enable format-on-save - Apply prettier configuration for whitespace remediation - Suppress new typescript warnings from newer TS version, to be fixed later
805146b
to
b2c15d8
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 updates dependency versions for TypeScript, ESLint, and typescript-eslint and migrates to an mjs configuration style to resolve linting errors. The changes include upgrading the Node.js version in the CI workflow, adding TS-expect-error annotations where newer TypeScript versions report issues, and refactoring some constructor signatures and icon helper functions.
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
eslint.config.mjs | Added new mjs ESLint configuration with TypeScript and Prettier settings. |
.github/workflows/build.yml | Updated Node.js version from 16.x to 20.x in the build workflow. |
src/pinnedWorkflows/pinnedWorkflows.ts | Added TS suppression comments for listing repo workflows and mapping workflow paths. |
src/store/workflowRun.ts | Added TS suppression comments for listing jobs for workflow runs and attempts. |
src/treeViews/settings/environmentSecretsNode.ts | Added TS suppression comments for listing environment secrets. |
src/treeViews/settings/repoSecretsNode.ts | Added TS suppression comments for listing repository secrets. |
src/treeViews/icons.ts | Updated icon helper function return types and signatures for workflow run and step icons. |
src/treeViews/settings/environmentVariablesNode.ts | Added TS suppression comments for listing environment variables. |
src/commands/openWorkflowFile.ts | Simplified error handling in the open workflow file command. |
src/treeViews/settings/repoVariablesNode.ts | Added TS suppression comments for listing repository variables. |
src/treeViews/shared/workflowJobNode.ts | Refactored constructor formatting for better readability. |
src/treeViews/shared/previousAttemptsNode.ts | Refactored constructor formatting for consistency. |
src/treeViews/shared/attemptNode.ts | Refactored constructor formatting to multi-line parameters. |
src/treeViews/current-branch/currentBranchRepoNode.ts | Refactored constructor formatting for clarity. |
src/treeViews/settings/environmentNode.ts | Refactored constructor formatting for uniformity. |
Comments suppressed due to low confidence (2)
src/pinnedWorkflows/pinnedWorkflows.ts:109
- [nitpick] Consider adding a reference to a tracked issue or more detailed context in the ts-expect-error comment to help maintainers know when this temporary suppression can be removed.
// @ts-expect-error FIXME: Newer Typescript catches a problem that previous didn't. This will be fixed in Octokit bump.
src/treeViews/icons.ts:87
- Changing the return value from an empty string to undefined in getIconForWorkflowStep might lead to unexpected behavior if downstream code expects a string; ensure that all consumers can handle an undefined value.
return undefined;
@joshmgross friendly bump :) |
Update Typescript, eslint, and typescript-eslint to latest version and switch to mjs style configuration.
This PR is a dependency for my other PRs to avoid linting errors.