initial round of PR feedback

josebalius · josebalius · commit 1e8a8370fee4 · 2021-08-27T16:29:02.000-04:00
diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go
@@ -108,7 +108,7 @@ func create(opts *createOptions) error {
 func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, user *api.User, codespace *api.Codespace) error {
 	states, err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace)
 	if err != nil {
-		return fmt.Errorf("poll post create states: %v", err)
+		return fmt.Errorf("failed to subscribe to state changes from codespace: %v", err)
 	}
 
 	var lastState codespaces.PostCreateState
@@ -135,27 +135,28 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, use
 					lastState = state
 					log.Print("...")
 					break
-				} else {
-					finishedStates[state.Name] = true
-					log.Println("..." + state.Status)
 				}
+
+				finishedStates[state.Name] = true
+				log.Println("..." + state.Status)
 			} else {
 				if state.Status == codespaces.PostCreateStateRunning {
 					inProgress = true
 					log.Print(".")
 					break
-				} else {
-					finishedStates[state.Name] = true
-					log.Println(state.Status)
-					lastState = codespaces.PostCreateState{} // reset the value
 				}
+
+				finishedStates[state.Name] = true
+				log.Println(state.Status)
+				lastState = codespaces.PostCreateState{} // reset the value
 			}
 		}
 
-		if !inProgress && !breakNextState {
+		if !inProgress {
+			if breakNextState {
+				break
+			}
 			breakNextState = true
-		} else if !inProgress && breakNextState {
-			break
 		}
 	}
 
diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go
@@ -62,14 +62,12 @@ type logger interface {
 	Println(v ...interface{}) (int, error)
 }
 
-// TODO(josebalius): we should move some of this to the liveshare.Connection struct
 func connectionReady(codespace *api.Codespace) bool {
-	ready := codespace.Environment.Connection.SessionID != ""
-	ready = ready && codespace.Environment.Connection.SessionToken != ""
-	ready = ready && codespace.Environment.Connection.RelayEndpoint != ""
-	ready = ready && codespace.Environment.Connection.RelaySAS != ""
-	ready = ready && codespace.Environment.State == api.CodespaceEnvironmentStateAvailable
-	return ready
+	return codespace.Environment.Connection.SessionID != "" &&
+		codespace.Environment.Connection.SessionToken != "" &&
+		codespace.Environment.Connection.RelayEndpoint != "" &&
+		codespace.Environment.Connection.RelaySAS != "" &&
+		codespace.Environment.State == api.CodespaceEnvironmentStateAvailable
 }
 
 func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, userLogin, token string, codespace *api.Codespace) (client *liveshare.Client, err error) {
@@ -82,8 +80,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use
 		}
 	}
 
-	retries := 0
-	for !connectionReady(codespace) {
+	for retries := 0; !connectionReady(codespace); retries++ {
 		if retries > 1 {
 			if retries%2 == 0 {
 				log.Print(".")
@@ -100,8 +97,6 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use
 		if err != nil {
 			return nil, fmt.Errorf("error getting codespace: %v", err)
 		}
-
-		retries += 1
 	}
 
 	if startedCodespace {
diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go
@@ -24,12 +24,10 @@ const (
 )
 
 type PostCreateStatesResult struct {
-	PostCreateStates PostCreateStates
+	PostCreateStates []PostCreateState
 	Err              error
 }
 
-type PostCreateStates []PostCreateState
-
 type PostCreateState struct {
 	Name   string                `json:"name"`
 	Status PostCreateStateStatus `json:"status"`
@@ -81,7 +79,7 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u
 	return pollch, nil
 }
 
-func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Codespace) (PostCreateStates, error) {
+func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Codespace) ([]PostCreateState, error) {
 	stdout, err := RunCommand(
 		ctx, tunnelPort, sshDestination(codespace),
 		"cat /workspaces/.codespaces/shared/postCreateOutput.json",
@@ -95,9 +93,9 @@ func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Cod
 		return nil, fmt.Errorf("read output: %v", err)
 	}
 
-	output := struct {
-		Steps PostCreateStates `json:"steps"`
-	}{}
+	var output struct {
+		Steps []PostCreateState `json:"steps"`
+	}
 	if err := json.Unmarshal(b, &output); err != nil {
 		return nil, fmt.Errorf("unmarshal output: %v", err)
 	}

Original file line number	Diff line number	Diff line change
`@@ -62,14 +62,12 @@ type logger interface {`
`62`	`62`	`Println(v ...interface{}) (int, error)`
`63`	`63`	`}`
`64`	`64`
`65`		`-// TODO(josebalius): we should move some of this to the liveshare.Connection struct`
`66`	`65`	`func connectionReady(codespace *api.Codespace) bool {`
`67`		`- ready := codespace.Environment.Connection.SessionID != ""`
`68`		`- ready = ready && codespace.Environment.Connection.SessionToken != ""`
`69`		`- ready = ready && codespace.Environment.Connection.RelayEndpoint != ""`
`70`		`- ready = ready && codespace.Environment.Connection.RelaySAS != ""`
`71`		`- ready = ready && codespace.Environment.State == api.CodespaceEnvironmentStateAvailable`
`72`		`- return ready`
	`66`	`+ return codespace.Environment.Connection.SessionID != "" &&`
	`67`	`+ codespace.Environment.Connection.SessionToken != "" &&`
	`68`	`+ codespace.Environment.Connection.RelayEndpoint != "" &&`
	`69`	`+ codespace.Environment.Connection.RelaySAS != "" &&`
	`70`	`+ codespace.Environment.State == api.CodespaceEnvironmentStateAvailable`
`73`	`71`	`}`
`74`	`72`
`75`	`73`	`func ConnectToLiveshare(ctx context.Context, log logger, apiClient api.API, userLogin, token string, codespace api.Codespace) (client *liveshare.Client, err error) {`
`@@ -82,8 +80,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use`
`82`	`80`	`}`
`83`	`81`	`}`
`84`	`82`
`85`		`- retries := 0`
`86`		`- for !connectionReady(codespace) {`
	`83`	`+ for retries := 0; !connectionReady(codespace); retries++ {`
`87`	`84`	`if retries > 1 {`
`88`	`85`	`if retries%2 == 0 {`
`89`	`86`	`log.Print(".")`
`@@ -100,8 +97,6 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use`
`100`	`97`	`if err != nil {`
`101`	`98`	`return nil, fmt.Errorf("error getting codespace: %v", err)`
`102`	`99`	`}`
`103`		`-`
`104`		`- retries += 1`
`105`	`100`	`}`
`106`	`101`
`107`	`102`	`if startedCodespace {`