-
Notifications
You must be signed in to change notification settings - Fork 214
Create base #1
Create base #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
main.go
Outdated
} | ||
|
||
flog.Info("starting code-server...") | ||
localPort := scanAvailablePort() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just use :0
to make ssh automatically use an available port. This is better than this as with this, a user on macOS will get a prompt asking if they want to let sshcode
listen on the network, and then get another prompt for the ssh binary which is confusing.
Not bad at all for first time Go btw, most of my issues are just nitpicks. |
83c9b42
to
38698a8
Compare
`, os.Args[0]) | ||
} | ||
|
||
flag.Parse() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker but in the future, you don't need the flag library, you have no flags. You can directly use os.Args
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nvm, flag automatically gives you a -h
flag which is nice. So this is the way to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure you run gofmt -s -w .
on the code.
Otherwise looks solid to me.
Congrats on passing your first @nhooyr software review 🎉 🔥
`, os.Args[0]) | ||
} | ||
|
||
flag.Parse() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nvm, flag automatically gives you a -h
flag which is nice. So this is the way to go.
var openCmd *exec.Cmd | ||
if commandExists("google-chrome") { | ||
openCmd = exec.Command("google-chrome", "--app="+url, "--disable-extensions", "--disable-plugins") | ||
} else if commandExists("firefox") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take it back. If neither of these exist, this program will panic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var openCmd *exec.Cmd | ||
if commandExists("google-chrome") { | ||
openCmd = exec.Command("google-chrome", "--app="+url, "--disable-extensions", "--disable-plugins") | ||
} else if commandExists("firefox") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💃
No description provided.