Skip to content

Conversation

@nektro
Copy link
Member

@nektro nektro commented Sep 19, 2025

user will still need to setup a valid ~/.aws/config on their own in order for these to execute properly

@robobun

This comment was marked as outdated.

@nektro nektro marked this pull request as ready for review September 19, 2025 01:51
@coderabbitai

This comment was marked as duplicate.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (4)
package.json (4)

87-90: Prefer LTS images (Ubuntu 24.04, Windows Server 2022) unless there’s a reason for interim/older releases.

If interim/older releases are intentional, add a short comment in machine.mjs or PR description.

Optional tweak:

-    "machine:linux:ubuntu": "bun ./scripts/machine.mjs ssh ... --distro=ubuntu --release=25.04",
+    "machine:linux:ubuntu": "bun ./scripts/machine.mjs ssh ... --distro=ubuntu --release=24.04",
-    "machine:windows:2019": "bun ./scripts/machine.mjs ssh ... --os=windows --release=2019",
+    "machine:windows:2022": "bun ./scripts/machine.mjs ssh ... --os=windows --release=2022",

87-90: Add ARM64 variants or make arch/instance-type configurable.

Many Bun CI/dev tasks run on arm64; consider c7g.* (Graviton) presets or env‑driven arch/instance type.

Examples to add (non-blocking):

"machine:linux:ubuntu:arm64": "bun ./scripts/machine.mjs ssh --cloud=aws --arch=arm64 --instance-type c7g.2xlarge --os=linux --distro=ubuntu --release=24.04",
"machine:linux:alpine:arm64": "bun ./scripts/machine.mjs ssh --cloud=aws --arch=arm64 --instance-type c7g.2xlarge --os=linux --distro=alpine --release=3.21",

Cost-safety: if machine.mjs supports it, wire in flags like --ttl/--auto-terminate/--tags to avoid orphaned instances.


87-90: Allow AWS profile/region override.

Developers often need non-default profiles/regions. Either add variants or read AWS_PROFILE/AWS_REGION inside machine.mjs.

Quick add (optional):

"machine:linux:ubuntu:prod": "AWS_PROFILE=prod bun ./scripts/machine.mjs ssh ...",

87-90: Name clarity and grouping nit.

Consider prefixing with machine:ssh:* to reflect the action and keep room for create/destroy/list later, e.g., machine:ssh:linux:ubuntu.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 987cab7 and feaa3a4.

📒 Files selected for processing (1)
  • package.json (1 hunks)

@nektro nektro requested a review from pfgithub September 20, 2025 00:23
@nektro nektro requested a review from dylan-conway September 20, 2025 00:51
@nektro nektro merged commit 13248ba into main Sep 24, 2025
10 of 12 checks passed
@nektro nektro deleted the nektro-patch-44083 branch September 24, 2025 20:11
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.

4 participants