Skip to content

Conversation

@sigmachirality
Copy link
Member

image image

@semanticdiff-com
Copy link

semanticdiff-com bot commented Dec 10, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  src/index.ts  17% smaller
  src/lib/vm/index.ts  9% smaller
  src/lib/vm/list.ts  0% smaller

@sigmachirality sigmachirality self-assigned this Dec 10, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 10, 2025

Greptile Overview

Greptile Summary

Added deprecation warnings to sf vm commands to nudge users toward sf nodes commands.

Key changes:

  • Added deprecation warning banner displayed on all sf vm commands via preAction hook and help text
  • Banner shows different warnings based on whether user has nodes with VMs
  • Updated sf vm list empty state message to reference sf nodes create --help
  • Made registerVM async to fetch node data for contextual warnings

Issues found:

  • Color wrapper functions (red(), yellow()) applied to strings with manual ANSI codes won't work as expected - the manual codes inside override the wrapper

Confidence Score: 3/5

  • This PR is safe to merge with minor color rendering issues
  • The deprecation warnings are well-implemented with appropriate user guidance, but ANSI color code conflicts will prevent colors from rendering correctly. The logic is sound but the visual presentation won't match intent.
  • src/lib/vm/index.ts needs color code fixes

Important Files Changed

File Analysis

Filename Score Overview
src/lib/vm/index.ts 3/5 Added deprecation warnings with ANSI color conflicts - wrapper functions don't apply when content already has manual color codes
src/lib/vm/list.ts 5/5 Updated copy to refer users to sf nodes create - clean change
src/index.ts 5/5 Changed registerVM to async call - correctly awaits the function

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI
    participant registerVM
    participant nodesClient
    participant getVMDeprecationWarning
    
    User->>CLI: sf vm [subcommand]
    CLI->>registerVM: await registerVM(program)
    registerVM->>registerVM: Setup command with preAction hook
    
    User->>CLI: Execute vm command
    CLI->>registerVM: preAction hook triggered
    registerVM->>getVMDeprecationWarning: await getVMDeprecationWarning()
    getVMDeprecationWarning->>nodesClient: await nodesClient()
    getVMDeprecationWarning->>nodesClient: client.nodes.list()
    nodesClient-->>getVMDeprecationWarning: nodes data
    
    alt User has nodes with VMs
        getVMDeprecationWarning-->>registerVM: yellow warning (custom message)
    else No nodes with VMs
        getVMDeprecationWarning-->>registerVM: red warning (generic deprecation)
    end
    
    registerVM->>User: console.error(warning)
    registerVM->>CLI: Continue with command execution
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@sigmachirality sigmachirality merged commit aac6c5e into main Dec 10, 2025
1 check passed
@sigmachirality sigmachirality deleted the dt/sf-vms-list-deprecated branch December 10, 2025 03:02
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.

2 participants