Clean up manual stage handing

Separate pushing the change to Gerrit from approval and staging, as suggested by Aapo.

Change-Id: I6d4c8f620c3df4503005baaf9fb2b5fac45076e6
Reviewed-by: Aapo Keskimolo <aapo.keskimolo@qt.io>
Reviewed-by: Frederik Gladhorn <frederik.gladhorn@qt.io>

3 files changed, 28 insertions, 12 deletions

diff --git a/src/qtmoduleupdater/gerrit.go b/src/qtmoduleupdater/gerrit.go
index 38101a6..2208255 100644
--- a/src/qtmoduleupdater/gerrit.go
+++ b/src/qtmoduleupdater/gerrit.go
@@ -205,7 +205,7 @@ func escapeGerritMessage(message string) string {
 	return `"` + replacer.Replace(message) + `"`
 }
 
-func pushAndStageChange(repoPath string, branch string, commitID OID, summary string, pushUserName string, manualStage bool) error {
+func pushChange(repoPath string, branch string, commitID OID, summary string, pushUserName string) error {
 	repo, err := OpenRepository(repoPath)
 	if err != nil {
 		return err
@@ -219,20 +219,25 @@ func pushAndStageChange(repoPath string, branch string, commitID OID, summary st
 		pushURL.User = url.User(pushUserName)
 	}
 
-	err = repo.Push(pushURL, commitID, "refs/for/"+branch)
+	return repo.Push(pushURL, commitID, "refs/for/"+branch)
+}
+
+func reviewAndStageChange(repoPath string, branch string, commitID OID, summary string, pushUserName string) error {
+	pushURL, err := RepoURL(repoPath)
 	if err != nil {
 		return err
 	}
+	if pushUserName != "" {
+		pushURL.User = url.User(pushUserName)
+	}
 
 	reviewArgs := []string{"gerrit", "review", string(commitID)}
 
 	if summary != "" {
 		reviewArgs = append(reviewArgs, "-m", escapeGerritMessage(summary))
 	}
-	if !manualStage {
-		// Pass in sanity review, since the sanity bot runs only after a delay and thus the commit will get refused.
-		reviewArgs = append(reviewArgs, "--code-review", "2", "--sanity-review", "1")
-	}
+	// Pass in sanity review, since the sanity bot runs only after a delay and thus the commit will get refused.
+	reviewArgs = append(reviewArgs, "--code-review", "2", "--sanity-review", "1")
 
 	updateCommand, err := gerritSSHCommand(*pushURL, reviewArgs...)
 	if err != nil {
@@ -244,10 +249,6 @@ func pushAndStageChange(repoPath string, branch string, commitID OID, summary st
 		return err
 	}
 
-	if manualStage {
-		return nil
-	}
-
 	stageArgs := []string{"gerrit-plugin-qt-workflow", "stage", string(commitID)}
 	updateCommand, err = gerritSSHCommand(*pushURL, stageArgs...)
 	if err != nil {
diff --git a/src/qtmoduleupdater/moduleupdatebatch.go b/src/qtmoduleupdater/moduleupdatebatch.go
index 384e4b4..067ec16 100644
--- a/src/qtmoduleupdater/moduleupdatebatch.go
+++ b/src/qtmoduleupdater/moduleupdatebatch.go
@@ -66,9 +66,16 @@ func (batch *ModuleUpdateBatch) scheduleUpdates(pushUserName string, manualStage
 			// Nothing to be done, we are waiting for indirect dependencies
 		} else if update.result == DependenciesUpdateUpdateScheduled {
 			// push and stage
-			if err = pushAndStageChange(moduleToUpdate.RepoPath, moduleToUpdate.Branch, update.commitID, update.summary, pushUserName, manualStage); err != nil {
+			if err = pushChange(moduleToUpdate.RepoPath, moduleToUpdate.Branch, update.commitID, update.summary, pushUserName); err != nil {
 				return fmt.Errorf("error pushing change upate: %s", err)
 			}
+
+			if !manualStage {
+				if err = reviewAndStageChange(moduleToUpdate.RepoPath, moduleToUpdate.Branch, update.commitID, update.summary, pushUserName); err != nil {
+					return fmt.Errorf("error pushing change upate: %s", err)
+				}
+			}
+
 			batch.Pending = append(batch.Pending, &PendingUpdate{moduleToUpdate, update.changeID})
 			delete(batch.Todo, moduleToUpdate.RepoPath)
 		} else {
diff --git a/src/qtmoduleupdater/qt5.go b/src/qtmoduleupdater/qt5.go
index 33efb05..1567878 100644
--- a/src/qtmoduleupdater/qt5.go
+++ b/src/qtmoduleupdater/qt5.go
@@ -246,5 +246,13 @@ func prepareQt5Update(product string, branch string, updatedModules map[string]*
 
 	fmt.Printf("Created new commit for submodule update: %s\n", commitOid)
 
-	return pushAndStageChange(product, branch, commitOid, "Updating all submodules with a new consistent set", pushUserName, manualStage)
+	if err = pushChange(product, branch, commitOid, "Updating all submodules with a new consistent set", pushUserName); err != nil {
+		return fmt.Errorf("Error pushing qt5 change: %s", err)
+	}
+
+	if manualStage {
+		return nil
+	}
+
+	return reviewAndStageChange(product, branch, commitOid, "Updating all submodules with a new consistent set", pushUserName)
 }