Skip to content

Conversation

@garrytrinder
Copy link
Member

Closes #212

@garrytrinder garrytrinder linked an issue May 20, 2025 that may be closed by this pull request
@garrytrinder garrytrinder requested a review from Copilot May 20, 2025 19:18
Copy link

Copilot AI left a 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 adds support for selecting the Dev Proxy Beta executable in all command handlers and bumps the extension version.

  • Replaces inline VersionExeName logic with a single getDevProxyExe(...) call across start/stop/restart/config commands
  • Updates package.json to version 0.23.1 and amends CHANGELOG.md with the new unreleased entry

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/commands.ts Introduce getDevProxyExe and remove VersionExeName branching logic
package.json Bump version to 0.23.1
CHANGELOG.md Update release header to 0.23.1 and add Beta support note
Comments suppressed due to low confidence (2)

src/commands.ts:89

  • No unit tests cover the dynamic executable selection for Dev Proxy Beta; consider adding tests for getDevProxyExe and the command handlers to verify correct behavior for both Stable and Beta versions.
const devProxyExe = getDevProxyExe(configuration.get('version') as VersionPreference);

src/commands.ts:254

  • [nitpick] Add a prompt or placeholder property to showInputBox to guide users on the expected filename format.
const fileName = await vscode.window.showInputBox({

@garrytrinder garrytrinder force-pushed the 212-update-start-command-with-support-for-beta-version branch from 4712a1e to af98206 Compare May 20, 2025 19:27
@garrytrinder garrytrinder requested a review from Copilot May 20, 2025 19:30
Copy link

Copilot AI left a 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 adds support for the Dev Proxy Beta by introducing a dynamic executable lookup and updating all commands to use it.

  • Import and use getDevProxyExe instead of hardcoded enum names throughout commands.
  • Update install, stop, restart, config-open, and config-new commands to reference devProxyExe.
  • Bump extension version and update changelog with Beta support entry.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/commands.ts Introduce devProxyExe via getDevProxyExe, remove VersionExeName usage
package.json Bump version from 0.23.0 to 0.23.1
CHANGELOG.md Update unreleased section to 0.23.1 and add Beta command support
Comments suppressed due to low confidence (2)

src/commands.ts:260

  • Remove this console.log statement in the validateInput callback—logging here appears to be leftover debugging code.
console.log(value);

src/commands.ts:9

  • Consider adding unit tests for getDevProxyExe and its usage in command registration, ensuring both Stable and Beta preferences produce the expected executable name.
const devProxyExe = getDevProxyExe(configuration.get('version') as VersionPreference);

import { isDevProxyRunning, getDevProxyExe } from './detect';

export const registerCommands = (context: vscode.ExtensionContext, configuration: vscode.WorkspaceConfiguration) => {
const versionPreference = configuration.get('version') as VersionPreference;
Copy link

Copilot AI May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The versionPreference variable is declared but never used; consider removing it or passing it directly to getDevProxyExe to avoid redundant configuration lookups.

Suggested change
const versionPreference = configuration.get('version') as VersionPreference;

Copilot uses AI. Check for mistakes.
@garrytrinder garrytrinder force-pushed the 212-update-start-command-with-support-for-beta-version branch from af98206 to e0a3ed9 Compare May 20, 2025 19:33
@garrytrinder garrytrinder requested a review from Copilot May 20, 2025 19:54
Copy link

Copilot AI left a 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 adds support for the Dev Proxy Beta by updating command execution to use a dynamic executable based on the configuration.

  • Updated command registration to fetch the correct executable path via getDevProxyExe.
  • Removed hardcoded executable names and redundant configuration lookups.
  • Updated package version and changelog to reflect these changes.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/commands.ts Replaced hardcoded executable names with a dynamic lookup using getDevProxyExe.
package.json Version bumped from 0.23.0 to 0.23.1.
CHANGELOG.md Updated changelog to reflect the new beta command support.

Comment on lines +9 to 14
const devProxyExe = getDevProxyExe(configuration.get('version') as VersionPreference);

context.subscriptions.push(
vscode.commands.registerCommand('dev-proxy-toolkit.install', async (platform: NodeJS.Platform) => {
const versionPreference = configuration.get('version') as VersionPreference;
const message = vscode.window.setStatusBarMessage('Installing Dev Proxy...');

Copy link

Copilot AI May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider whether configuration changes at runtime should update devProxyExe dynamically. If configuration may change after activation, it might be better to compute devProxyExe inside each command handler.

Copilot uses AI. Check for mistakes.
@garrytrinder garrytrinder merged commit 09f8c52 into main May 20, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update commands with support for beta version

2 participants