Skip to content

Rename sf script to sf cloud-init user-data #96

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

coffinsfcompute
Copy link
Contributor

@coffinsfcompute coffinsfcompute commented Mar 21, 2025

Rename for clarity. Also adds sf vm cloud-init user-data get. Feel free to give suggestions on renaming.

Copy link

semanticdiff-com bot commented Mar 21, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  src/lib/vm.ts  8% smaller

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.

PR Summary

Renamed the 'script' command to 'cloud-init user-data' in the VM management CLI, improving clarity and organization of cloud initialization configuration commands.

  • Added new cloud-init subcommand group in src/lib/vm.ts to better organize VM initialization settings
  • Added get command to retrieve existing cloud-init user-data configuration
  • Updated error messages to use consistent "cloud-init user-data" terminology
  • Maintained backward compatibility by using same API endpoints (vms_script_post/get)

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +21 to +29
const url = await getApiUrl("vms_script_post");
const response = await fetch(url, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${await getAuthToken()}`,
},
body: JSON.stringify({ script: userData }),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add spinner. you can prompt cursor to do this.

.description("Push a startup script to VMs")
.requiredOption("-f, --file <file>", "Path to startup script file")
vm.command("script").description(
"OBSOLETE - Now an alias for `sf vm cloud-init user-data set`",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"OBSOLETE - Now an alias for `sf vm cloud-init user-data set`",
"Deprecated - Now an alias for `sf vm cloud-init user-data set`",

} catch {
logAndQuit("Failed to read script file");
}
console.error("OBSOLETE - Please use `sf vm cloud-init user-data set`.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.error("OBSOLETE - Please use `sf vm cloud-init user-data set`.");
console.error("Deprecate - Use `sf vm cloud-init user-data set`.");

@coffinsfcompute
Copy link
Contributor Author

I realized I never added context for why this hasn't been merged. Evan didn't like this so I'm leaving it unmerged until someone can do the work of refactoring our cloud UX.

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