-
Notifications
You must be signed in to change notification settings - Fork 18
Add coder envs rebuild and watch-build commands #146
Conversation
cmoog
commented
Oct 20, 2020
•
edited
Loading
edited
// BuildLogFollowMsg wraps the base BuildLog and adds a field for collecting | ||
// errors that may occur when follow or parsing. | ||
type BuildLogFollowMsg struct { | ||
BuildLog | ||
Err error | ||
} | ||
|
||
// FollowEnvironmentBuildLog trails the build log of a Coder environment. | ||
func (c Client) FollowEnvironmentBuildLog(ctx context.Context, envID string) (<-chan BuildLogFollowMsg, error) { | ||
ch := make(chan BuildLogFollowMsg) | ||
ws, err := c.DialEnvironmentBuildLog(ctx, envID) |
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.
return l.Err | ||
} | ||
switch l.BuildLog.Type { | ||
case coder.BuildLogTypeStart: |
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.
Do we need to do anything for this case?
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.
The FE uses this to reset the UI logs, but since we're just trailing I don't think it makes sense to do anything. Plus we exit after the Done message is sent anyway.
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.
That makes sense.
I think that explanation should be plugged in as a comment.
internal/cmd/rebuild.go
Outdated
fmt.Print(color.RedString("\t%s", l.BuildLog.Msg)) | ||
s = newSpinner() | ||
case coder.BuildLogTypeDone: | ||
s.Stop() |
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.
Is it possible that s
could be nil here or is it safe to assume we'll always have assigned the return value of newSpinner
to s
before we get here?
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.
Ah good catch. Assuming the build log records are properly formed, this shouldn't be an issue. That said, I'll push a change that better handles the nil case to avoid panics.
077cfa8
to
a14543a
Compare