refactor(submit): Forge::update returns UpdateStatuses

For uniformity with `CreateStatus`.

Author: arxanas

git-branchless-submit/src/branch_forge.rs

@@ -15,7 +15,7 @@ use lib::try_exit_code;
 use lib::util::{ExitCode, EyreExitOr};
 use tracing::{instrument, warn};
 
-use crate::{CommitStatus, CreateStatus, Forge, SubmitOptions, SubmitStatus};
+use crate::{CommitStatus, CreateStatus, Forge, SubmitOptions, SubmitStatus, UpdateStatus};
 
 #[derive(Debug)]
 pub struct BranchForge<'a> {
@@ -269,20 +269,20 @@ These remotes are available: {}",
         &mut self,
         commits: HashMap<NonZeroOid, CommitStatus>,
         _options: &SubmitOptions,
-    ) -> EyreExitOr<()> {
-        let branches_by_remote: BTreeMap<String, BTreeSet<String>> = commits
-            .into_values()
-            .flat_map(|commit_status| match commit_status {
+    ) -> EyreExitOr<HashMap<NonZeroOid, UpdateStatus>> {
+        let branches_by_remote: BTreeMap<String, BTreeSet<(String, NonZeroOid)>> = commits
+            .into_iter()
+            .flat_map(|(commit_oid, commit_status)| match commit_status {
                 CommitStatus {
                     submit_status: _,
                     remote_name: Some(remote_name),
                     local_commit_name: Some(local_commit_name),
                     remote_commit_name: _,
-                } => Some((remote_name, local_commit_name)),
+                } => Some((remote_name, (local_commit_name, commit_oid))),
                 commit_status => {
                     warn!(
                         ?commit_status,
-                        "Commit was requested to be updated, but it did not have the requisite information (remote name, local branch name)."
+                        "Commit was requested to be updated, but it did not have an associated remote and a local commit name"
                     );
                     None
                 }
@@ -299,24 +299,39 @@ These remotes are available: {}",
             .values()
             .map(|branch_names| branch_names.len())
             .sum();
+        let mut result = HashMap::new();
         progress.notify_progress(0, total_num_branches);
         for (remote_name, branch_names) in branches_by_remote {
             let mut args = vec!["push", "--force-with-lease", &remote_name];
-            args.extend(branch_names.iter().map(|s| s.as_str()));
+            args.extend(
+                branch_names
+                    .iter()
+                    .map(|(branch, _commit_oid)| branch.as_str()),
+            );
             match self.git_run_info.run(&effects, Some(event_tx_id), &args)? {
                 Ok(()) => {}
                 Err(exit_code) => {
                     writeln!(
                         effects.get_output_stream(),
                         "Failed to push branches: {}",
-                        branch_names.into_iter().join(", ")
+                        branch_names
+                            .iter()
+                            .map(|(branch, _commit_oid)| branch)
+                            .join(", ")
                     )?;
                     return Ok(Err(exit_code));
                 }
             }
             progress.notify_progress_inc(branch_names.len());
+
+            // FIXME: report push errors
+            result.extend(
+                branch_names
+                    .iter()
+                    .map(|(_branch, commit_oid)| (*commit_oid, UpdateStatus::Updated)),
+            );
         }
 
-        Ok(Ok(()))
+        Ok(Ok(result))
     }
 }

git-branchless-submit/src/github.rs

@@ -33,6 +33,7 @@ use tracing::warn;
 
 use crate::branch_forge::BranchForge;
 use crate::SubmitStatus;
+use crate::UpdateStatus;
 use crate::{CommitStatus, CreateStatus, Forge, SubmitOptions};
 
 /// Testing environment variable. When this is set, the executable will use the
@@ -370,7 +371,7 @@ impl Forge for GithubForge<'_> {
         &mut self,
         commit_statuses: HashMap<NonZeroOid, CommitStatus>,
         options: &SubmitOptions,
-    ) -> EyreExitOr<()> {
+    ) -> EyreExitOr<HashMap<NonZeroOid, UpdateStatus>> {
         let effects = self.effects;
         let SubmitOptions {
             create: _,
@@ -394,6 +395,7 @@ impl Forge for GithubForge<'_> {
 
         let commit_set: CommitSet = commit_statuses.keys().copied().collect();
         let commit_oids = self.dag.sort(&commit_set)?;
+        let mut result = HashMap::new();
         {
             let (effects, progress) = effects.start_operation(OperationType::UpdateCommits);
             progress.notify_progress(0, commit_oids.len());
@@ -474,7 +476,7 @@ impl Forge for GithubForge<'_> {
                     branch_forge.update(singleton(&commit_statuses, commit_oid, |x| x), options)?
                 );
 
-                // Update metdata:
+                // Update metadata:
                 try_exit_code!(self.client.update_pull_request(
                     &effects,
                     pull_request_info.number,
@@ -487,10 +489,13 @@ impl Forge for GithubForge<'_> {
                     options
                 )?);
                 progress.notify_progress_inc(1);
+
+                // FIXME: report push/update errors
+                result.insert(commit_oid, UpdateStatus::Updated);
             }
         }
 
-        Ok(Ok(()))
+        Ok(Ok(result))
     }
 }
 

git-branchless-submit/src/lib.rs

@@ -171,6 +171,15 @@ pub enum CreateStatus {
     },
 }
 
+/// The result of updating a commit.
+///
+/// TODO: have an `Err` case.
+#[derive(Clone, Debug)]
+pub enum UpdateStatus {
+    /// The commit was successfully updated.
+    Updated,
+}
+
 /// "Forge" refers to a Git hosting provider, such as GitHub, GitLab, etc.
 /// Commits can be pushed for review to a forge.
 pub trait Forge: Debug {
@@ -192,7 +201,7 @@ pub trait Forge: Debug {
         &mut self,
         commits: HashMap<NonZeroOid, CommitStatus>,
         options: &SubmitOptions,
-    ) -> EyreExitOr<()>;
+    ) -> EyreExitOr<HashMap<NonZeroOid, UpdateStatus>>;
 }
 
 /// `submit` command.

git-branchless-submit/src/phabricator.rs

@@ -35,7 +35,9 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 use tracing::{instrument, warn};
 
-use crate::{CommitStatus, CreateStatus, Forge, SubmitOptions, SubmitStatus, STYLE_PUSHED};
+use crate::{
+    CommitStatus, CreateStatus, Forge, SubmitOptions, SubmitStatus, UpdateStatus, STYLE_PUSHED,
+};
 
 /// Wrapper around the Phabricator "ID" type. (This is *not* a PHID, just a
 /// regular ID).
@@ -614,7 +616,7 @@ fi
         &mut self,
         commits: HashMap<NonZeroOid, crate::CommitStatus>,
         options: &SubmitOptions,
-    ) -> EyreExitOr<()> {
+    ) -> EyreExitOr<HashMap<NonZeroOid, UpdateStatus>> {
         let SubmitOptions {
             create: _,
             draft: _,
@@ -740,12 +742,24 @@ fi
 
         try_exit_code!(self.update_dependencies(
             &success_commits
-                .into_iter()
+                .iter()
                 .map(|(commit_oid, _test_output)| commit_oid)
+                .copied()
                 .collect(),
             &CommitSet::empty()
         )?);
-        Ok(Ok(()))
+        Ok(Ok(success_commits
+            .into_iter()
+            .map(|(commit_oid, _test_output)| (commit_oid, UpdateStatus::Updated))
+            .chain(
+                failure_commits
+                    .into_iter()
+                    .map(|(commit_oid, _test_output)| {
+                        // FIXME: report error
+                        (commit_oid, UpdateStatus::Updated)
+                    }),
+            )
+            .collect()))
     }
 }