fix(workflow): add missing runner types and remove deprecated #86#94
Merged
fix(workflow): add missing runner types and remove deprecated #86#94
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduce a shared RUNNER_OPTIONS module with types and tests, switch JobPropertyPanel (both host and webview) to derive runner dropdown entries/icons from RUNNER_OPTIONS, expand CI matrix OS values with new variants and remove deprecated entries, and include the shared lib in the webview tsconfig. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/JobPropertyPanel.tsx (1)
114-133: Consider centralizing runner labels to avoid future drift.
runnerOptionshere and insrc/webview/components/JobPropertyPanel.tsxare duplicated literals. Extracting a shared runner-label source would prevent future partial updates like this PR’s Linux mismatch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/JobPropertyPanel.tsx` around lines 114 - 133, runnerOptions is duplicated across JobPropertyPanel components causing drift; extract a single shared constant (e.g., RUNNER_OPTIONS or RUNNER_LABELS) exported from a new shared module (e.g., runners.ts) and replace the local runnerOptions arrays in both JobPropertyPanel components with an import. Ensure the shared constant contains canonical value/label/icon tuples (matching getLinuxIcon/getMacIcon/getWindowsIcon/getServerIcon usage) and update any consumers (map/filter) to use the imported constant so future updates are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/JobPropertyPanel.tsx`:
- Around line 115-121: In JobPropertyPanel (the runner options array) replace
the deprecated 'ubuntu-20.04' entry with the missing x64 'ubuntu-24.04' entry
and ensure the ARM variant remains as 'ubuntu-24.04-arm'; update the option
objects so they include the correct value strings ('ubuntu-24.04' and
'ubuntu-24.04-arm') and labels (e.g., "Ubuntu 24.04" and "Ubuntu 24.04 (ARM)")
and keep using getLinuxIcon() for both so UI selection reflects supported
GitHub-hosted runners.
In `@src/lib/matrixOptions.ts`:
- Around line 27-33: The matrix array in src/lib/matrixOptions.ts includes
deprecated and missing GitHub runner labels: remove 'ubuntu-20.04' (no longer
supported) and add the missing x64 image 'ubuntu-24.04' so the matrix contains
the current supported set (e.g., keep
'ubuntu-latest','ubuntu-slim','ubuntu-24.04','ubuntu-24.04-arm','ubuntu-22.04','ubuntu-22.04-arm','windows-latest');
update the array where those string literals appear (the list that currently
contains 'ubuntu-20.04' and only 'ubuntu-24.04-arm') accordingly.
---
Nitpick comments:
In `@src/components/JobPropertyPanel.tsx`:
- Around line 114-133: runnerOptions is duplicated across JobPropertyPanel
components causing drift; extract a single shared constant (e.g., RUNNER_OPTIONS
or RUNNER_LABELS) exported from a new shared module (e.g., runners.ts) and
replace the local runnerOptions arrays in both JobPropertyPanel components with
an import. Ensure the shared constant contains canonical value/label/icon tuples
(matching getLinuxIcon/getMacIcon/getWindowsIcon/getServerIcon usage) and update
any consumers (map/filter) to use the imported constant so future updates are
made in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a25434f-edbe-4fc0-8101-f4c31cce92d4
⛔ Files ignored due to path filters (1)
media/main.js.mapis excluded by!**/*.map
📒 Files selected for processing (5)
media/main.jssrc/components/JobPropertyPanel.tsxsrc/lib/matrixOptions.tssrc/webview/components/JobPropertyPanel.tsxsrc/webview/lib/matrixOptions.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #94 +/- ##
==========================================
+ Coverage 93.68% 93.77% +0.09%
==========================================
Files 8 9 +1
Lines 269 273 +4
Branches 101 101
==========================================
+ Hits 252 256 +4
Misses 1 1
Partials 16 16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/lib/runnerConfig.test.ts (2)
2-2: Use the@/alias for this app-code import.Switching this import to alias form keeps consistency with repo import conventions.
♻️ Suggested change
-import { RUNNER_OPTIONS } from './runnerConfig' +import { RUNNER_OPTIONS } from '@/lib/runnerConfig'As per coding guidelines "Prefer
@/for app code:import { parseWorkflow } from '@/lib/parseWorkflow'."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/runnerConfig.test.ts` at line 2, Update the import in runnerConfig.test.ts to use the app-code alias: replace the current import of RUNNER_OPTIONS from './runnerConfig' with the '@/lib/runnerConfig' alias so the test follows the repo convention; ensure the imported symbol RUNNER_OPTIONS remains unchanged and any relative path usages in this file are converted to the '@/...' form.
5-13: Strengthen assertions to lock PR runner additions/removals.Current checks cover a subset. Add explicit assertions for newly introduced values and explicit
not.toContainchecks for removed deprecated runners to prevent regressions.🧪 Suggested test expansion
it('includes common runners', () => { const values = RUNNER_OPTIONS.map((opt) => opt.value) expect(values).toContain('ubuntu-latest') + expect(values).toContain('ubuntu-slim') expect(values).toContain('ubuntu-24.04') expect(values).toContain('ubuntu-24.04-arm') + expect(values).toContain('ubuntu-22.04-arm') + expect(values).toContain('windows-2025') + expect(values).toContain('windows-2025-vs2026') + expect(values).toContain('windows-11-arm') + expect(values).toContain('macos-15') + expect(values).toContain('macos-15-intel') + expect(values).toContain('macos-26') + expect(values).toContain('macos-26-intel') expect(values).toContain('macos-latest') expect(values).toContain('windows-latest') expect(values).toContain('self-hosted') }) + + it('does not include deprecated runners', () => { + const values = RUNNER_OPTIONS.map((opt) => opt.value) + expect(values).not.toContain('ubuntu-20.04') + expect(values).not.toContain('windows-2019') + expect(values).not.toContain('macos-13') + expect(values).not.toContain('macos-12') + })Also applies to: 23-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/runnerConfig.test.ts` around lines 5 - 13, The test "includes common runners" only asserts a subset of runners; update it to explicitly assert presence of all allowed PR runner values and explicitly assert absence of any deprecated/removed runners to prevent accidental additions/removals. Locate RUNNER_OPTIONS in the test, compute const values = RUNNER_OPTIONS.map((opt) => opt.value), then add expect(values).toContain('ubuntu-latest'), expect(values).toContain('ubuntu-24.04'), expect(values).toContain('ubuntu-24.04-arm'), expect(values).toContain('macos-latest'), expect(values).toContain('windows-latest'), expect(values).toContain('self-hosted') plus any newly introduced runner strings added in the diff, and add expect(values).not.toContain('<deprecated-runner-1>') etc. to explicitly forbid removed/deprecated runners.src/lib/runnerConfig.ts (1)
15-34: MakeRUNNER_OPTIONSimmutable to protect the shared source of truth.Because this array is exported and reused across panels, leaving it mutable increases drift risk from accidental runtime mutation.
♻️ Suggested change
-export const RUNNER_OPTIONS: RunnerOptionConfig[] = [ +export const RUNNER_OPTIONS: ReadonlyArray<RunnerOptionConfig> = [ { value: 'ubuntu-latest', label: 'Ubuntu Latest', iconType: 'linux' }, { value: 'ubuntu-slim', label: 'Ubuntu Slim', iconType: 'linux' }, { value: 'ubuntu-24.04', label: 'Ubuntu 24.04', iconType: 'linux' }, { value: 'ubuntu-24.04-arm', label: 'Ubuntu 24.04 (ARM)', iconType: 'linux' }, { value: 'ubuntu-22.04', label: 'Ubuntu 22.04', iconType: 'linux' }, { value: 'ubuntu-22.04-arm', label: 'Ubuntu 22.04 (ARM)', iconType: 'linux' }, { value: 'macos-latest', label: 'macOS Latest', iconType: 'mac' }, { value: 'macos-26', label: 'macOS 26', iconType: 'mac' }, { value: 'macos-26-intel', label: 'macOS 26 (Intel)', iconType: 'mac' }, { value: 'macos-15', label: 'macOS 15', iconType: 'mac' }, { value: 'macos-15-intel', label: 'macOS 15 (Intel)', iconType: 'mac' }, { value: 'macos-14', label: 'macOS 14', iconType: 'mac' }, { value: 'windows-latest', label: 'Windows Latest', iconType: 'windows' }, { value: 'windows-2025', label: 'Windows 2025', iconType: 'windows' }, { value: 'windows-2025-vs2026', label: 'Windows 2025 VS 2026 (preview)', iconType: 'windows' }, { value: 'windows-11-arm', label: 'Windows 11 (ARM)', iconType: 'windows' }, { value: 'windows-2022', label: 'Windows 2022', iconType: 'windows' }, { value: 'self-hosted', label: 'Self-hosted', iconType: 'server' }, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/runnerConfig.ts` around lines 15 - 34, Make RUNNER_OPTIONS immutable: change its type to a readonly form (e.g., ReadonlyArray<Readonly<RunnerOptionConfig>>) and freeze both the array and each option object so callers cannot mutate runtime values; update the export of RUNNER_OPTIONS to construct the array and then Object.freeze every entry and the array itself (reference RUNNER_OPTIONS and the RunnerOptionConfig items to find where to apply the freezes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/runnerConfig.test.ts`:
- Line 2: Update the import in runnerConfig.test.ts to use the app-code alias:
replace the current import of RUNNER_OPTIONS from './runnerConfig' with the
'@/lib/runnerConfig' alias so the test follows the repo convention; ensure the
imported symbol RUNNER_OPTIONS remains unchanged and any relative path usages in
this file are converted to the '@/...' form.
- Around line 5-13: The test "includes common runners" only asserts a subset of
runners; update it to explicitly assert presence of all allowed PR runner values
and explicitly assert absence of any deprecated/removed runners to prevent
accidental additions/removals. Locate RUNNER_OPTIONS in the test, compute const
values = RUNNER_OPTIONS.map((opt) => opt.value), then add
expect(values).toContain('ubuntu-latest'),
expect(values).toContain('ubuntu-24.04'),
expect(values).toContain('ubuntu-24.04-arm'),
expect(values).toContain('macos-latest'),
expect(values).toContain('windows-latest'),
expect(values).toContain('self-hosted') plus any newly introduced runner strings
added in the diff, and add expect(values).not.toContain('<deprecated-runner-1>')
etc. to explicitly forbid removed/deprecated runners.
In `@src/lib/runnerConfig.ts`:
- Around line 15-34: Make RUNNER_OPTIONS immutable: change its type to a
readonly form (e.g., ReadonlyArray<Readonly<RunnerOptionConfig>>) and freeze
both the array and each option object so callers cannot mutate runtime values;
update the export of RUNNER_OPTIONS to construct the array and then
Object.freeze every entry and the array itself (reference RUNNER_OPTIONS and the
RunnerOptionConfig items to find where to apply the freezes).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8fa03162-aab4-4660-a409-40e88c68bd18
⛔ Files ignored due to path filters (1)
media/main.js.mapis excluded by!**/*.map
📒 Files selected for processing (6)
media/main.jssrc/components/JobPropertyPanel.tsxsrc/lib/runnerConfig.test.tssrc/lib/runnerConfig.tssrc/webview/components/JobPropertyPanel.tsxtsconfig.webview.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/runnerConfig.ts`:
- Line 35: The forEach callback currently uses an expression-bodied arrow that
implicitly returns Object.freeze(opt), which violates the
no-return-in-iterable-callback rule; change the callback to a block-bodied
function so it does not return a value (e.g., replace
runnerOptions.forEach((opt) => Object.freeze(opt)) with a block callback that
calls Object.freeze(opt);), keeping the same runnerOptions and Object.freeze
usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88b87e91-83a7-49a0-9e65-b478cc2ba445
⛔ Files ignored due to path filters (1)
media/main.js.mapis excluded by!**/*.map
📒 Files selected for processing (3)
media/main.jssrc/lib/runnerConfig.test.tssrc/lib/runnerConfig.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/runnerConfig.test.ts
🩺 React Doctor |
|
🎉 This PR is included in version 1.2.38 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Update the following runner types that are missing:
Remove the following deprecated runner types:
Type of change
Checklist
Code quality
feat:,fix:,docs:,chore:)anyTypeScript types without justificationconsole.log, or commented-out blocksTesting
F5in VSCode to launch the Extension Development Hostpnpm test, and all tests passpnpm lint, and there are no lint errorsBuild & compatibility
pnpm run compileandpnpm run webpackwithout errorsDocumentation
README.mdif my change adds a new feature, keyboard shortcut, or changes existing behaviourScreenshots/recordings
Summary by CodeRabbit