- Notifications
You must be signed in to change notification settings - Fork18
Add coder envs rebuild and watch-build commands#146
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| // 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 thats could be nil here or is it safe to assume we'll always have assigned the return value ofnewSpinner tos 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, thisshouldn't be an issue. That said, I'll push a change that better handles the nil case to avoid panics.
Uh oh!
There was an error while loading.Please reload this page.
077cfa8 toa14543aCompare
Uh oh!
There was an error while loading.Please reload this page.