From ee9ee691306a8accc627a00835a3116e9d235814 Mon Sep 17 00:00:00 2001
From: Fintan Halpenny <fintan.halpenny@gmail.com>
Date: Fri, 1 Dec 2023 12:02:38 +0000
Subject: [PATCH] cli: move rad review to rad patch

It is more natural for the review command to exist as a subcommand of
patch, since the reviews are for patches.

Remove the `rad review` command, moving its functionality to the `rad
patch` command. The rad-patch.md and rad-review-by-hunk.md tests are
modified to use the `rad patch review` command.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
---
 radicle-cli/examples/rad-patch.md             |   2 +-
 radicle-cli/examples/rad-review-by-hunk.md    |  14 +-
 radicle-cli/src/commands.rs                   |   2 -
 radicle-cli/src/commands/help.rs              |   1 -
 radicle-cli/src/commands/patch.rs             | 104 ++++++
 radicle-cli/src/commands/patch/review.rs      | 141 +++++++++
 .../commands/{ => patch}/review/builder.rs    |   0
 radicle-cli/src/commands/review.rs            | 299 ------------------
 radicle-cli/src/main.rs                       |   7 -
 9 files changed, 253 insertions(+), 317 deletions(-)
 create mode 100644 radicle-cli/src/commands/patch/review.rs
 rename radicle-cli/src/commands/{ => patch}/review/builder.rs (100%)
 delete mode 100644 radicle-cli/src/commands/review.rs

diff --git a/radicle-cli/examples/rad-patch.md b/radicle-cli/examples/rad-patch.md
index 896675c1..64223e82 100644
--- a/radicle-cli/examples/rad-patch.md
+++ b/radicle-cli/examples/rad-patch.md
@@ -154,7 +154,7 @@ $ rad patch checkout 6ff4f09
 We can also add a review verdict as such:
 
 ```
-$ rad review 6ff4f09 --accept --no-message --no-sync
+$ rad patch review 6ff4f09 --accept --no-message
 ✓ Patch 6ff4f09 accepted
 ```
 
diff --git a/radicle-cli/examples/rad-review-by-hunk.md b/radicle-cli/examples/rad-review-by-hunk.md
index 047cbcc8..53117104 100644
--- a/radicle-cli/examples/rad-review-by-hunk.md
+++ b/radicle-cli/examples/rad-review-by-hunk.md
@@ -70,7 +70,7 @@ Finally, we do a review of the patch by hunk. The output of this command should
 match `git diff master -W100% -U5 --patience`:
 
 ```
-$ rad review --no-sync --patch -U5 52da8356aa9beec08e6943cb3c4063fa37f3241b
+$ rad patch review --patch -U5 52da8356aa9beec08e6943cb3c4063fa37f3241b
 diff --git a/.gitignore b/.gitignore
 deleted file mode 100644
 index 7937fb3..0000000
@@ -116,7 +116,7 @@ rename to notes/INSTRUCTIONS.txt
 Now let's accept these hunks one by one..
 
 ```
-$ rad review --no-sync --patch --accept --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
+$ rad patch review --patch --accept --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
 ✓ Loaded existing review ([..]) for patch 52da8356aa9beec08e6943cb3c4063fa37f3241b
 diff --git a/.gitignore b/.gitignore
 deleted file mode 100644
@@ -127,7 +127,7 @@ index 7937fb3..0000000
 -*.draft
 ```
 ```
-$ rad review --no-sync --patch --accept --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
+$ rad patch review --patch --accept --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
 ✓ Loaded existing review ([..]) for patch 52da8356aa9beec08e6943cb3c4063fa37f3241b
 diff --git a/DISCLAIMER.txt b/DISCLAIMER.txt
 new file mode 100644
@@ -138,7 +138,7 @@ index 0000000..2b5bd86
 +All food is served as-is, with no warranty!
 ```
 ```
-$ rad review --no-sync --patch --accept -U3 --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
+$ rad patch review --patch --accept -U3 --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
 ✓ Loaded existing review ([..]) for patch 52da8356aa9beec08e6943cb3c4063fa37f3241b
 diff --git a/MENU.txt b/MENU.txt
 index 867958c..3af9741 100644
@@ -153,7 +153,7 @@ index 867958c..3af9741 100644
 [..]
 ```
 ```
-$ rad review --no-sync --patch --accept -U3 --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
+$ rad patch review --patch --accept -U3 --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
 ✓ Loaded existing review ([..]) for patch 52da8356aa9beec08e6943cb3c4063fa37f3241b
 diff --git a/MENU.txt b/MENU.txt
 index 4e2e828..3af9741 100644
@@ -169,7 +169,7 @@ index 4e2e828..3af9741 100644
 ```
 
 ```
-$ rad review --no-sync --patch --accept --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
+$ rad patch review --patch --accept --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
 ✓ Loaded existing review ([..]) for patch 52da8356aa9beec08e6943cb3c4063fa37f3241b
 diff --git a/INSTRUCTIONS.txt b/notes/INSTRUCTIONS.txt
 similarity index 100%
@@ -178,7 +178,7 @@ rename to notes/INSTRUCTIONS.txt
 ```
 
 ```
-$ rad review --no-sync --patch --accept --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
+$ rad patch review --patch --accept --hunk 1 52da8356aa9beec08e6943cb3c4063fa37f3241b
 ✓ Loaded existing review ([..]) for patch 52da8356aa9beec08e6943cb3c4063fa37f3241b
 ✓ All hunks have been reviewed
 ```
diff --git a/radicle-cli/src/commands.rs b/radicle-cli/src/commands.rs
index c1c74d12..0deabc29 100644
--- a/radicle-cli/src/commands.rs
+++ b/radicle-cli/src/commands.rs
@@ -36,8 +36,6 @@ pub mod rad_path;
 pub mod rad_publish;
 #[path = "commands/remote.rs"]
 pub mod rad_remote;
-#[path = "commands/review.rs"]
-pub mod rad_review;
 #[path = "commands/self.rs"]
 pub mod rad_self;
 #[path = "commands/sync.rs"]
diff --git a/radicle-cli/src/commands/help.rs b/radicle-cli/src/commands/help.rs
index 7a99e3fc..41efe4e6 100644
--- a/radicle-cli/src/commands/help.rs
+++ b/radicle-cli/src/commands/help.rs
@@ -26,7 +26,6 @@ const COMMANDS: &[Help] = &[
     rad_node::HELP,
     rad_patch::HELP,
     rad_path::HELP,
-    rad_review::HELP,
     rad_clean::HELP,
     rad_self::HELP,
     rad_track::HELP,
diff --git a/radicle-cli/src/commands/patch.rs b/radicle-cli/src/commands/patch.rs
index 637165c8..e6599f61 100644
--- a/radicle-cli/src/commands/patch.rs
+++ b/radicle-cli/src/commands/patch.rs
@@ -20,6 +20,8 @@ mod list;
 mod ready;
 #[path = "patch/redact.rs"]
 mod redact;
+#[path = "patch/review.rs"]
+mod review;
 #[path = "patch/show.rs"]
 mod show;
 #[path = "patch/update.rs"]
@@ -53,6 +55,7 @@ Usage
     rad patch archive <patch-id> [<option>...]
     rad patch update <patch-id> [<option>...]
     rad patch checkout <patch-id> [<option>...]
+    rad patch review <patch-id> [--accept | --reject] [-m [<string>]] [-d | --delete] [<option>...]
     rad patch delete <patch-id> [<option>...]
     rad patch redact <revision-id> [<option>...]
     rad patch assign <revision-id> [--add <did>] [--delete <did>] [<option>...]
@@ -75,6 +78,18 @@ Edit options
 
     -m, --message [<string>]   Provide a comment message to the patch or revision (default: prompt)
 
+Review options
+
+        --revision <id>        Review the given revision of the patch
+    -p, --patch                Review by patch hunks
+        --hunk <index>         Only review a specific hunk
+        --accept               Accept a patch or set of hunks
+        --reject               Reject a patch or set of hunks
+    -U, --unified <n>          Generate diffs with <n> lines of context instead of the usual three
+    -d, --delete               Delete a review draft
+    -r, --revision             Review a patch revision
+    -m, --message [<string>]   Provide a comment with the review (default: prompt)
+
 Assign options
 
     -a, --add    <did>         Add an assignee to the patch (may be specified multiple times).
@@ -134,6 +149,7 @@ pub enum OperationName {
     Checkout,
     Comment,
     Ready,
+    Review,
     Label,
     #[default]
     List,
@@ -205,6 +221,11 @@ pub enum Operation {
         message: Message,
         reply_to: Option<Rev>,
     },
+    Review {
+        patch_id: Rev,
+        revision_id: Option<Rev>,
+        opts: review::Options,
+    },
     Assign {
         patch_id: Rev,
         opts: AssignOptions,
@@ -261,6 +282,7 @@ impl Args for Options {
         let mut checkout_opts = checkout::Options::default();
         let mut assign_opts = AssignOptions::default();
         let mut label_opts = LabelOptions::default();
+        let mut review_op = review::Operation::default();
 
         while let Some(arg) = parser.next()? {
             match arg {
@@ -314,6 +336,65 @@ impl Args for Options {
                     reply_to = Some(rev);
                 }
 
+                // Review options.
+                Long("revision") if op == Some(OperationName::Review) => {
+                    let val = parser.value()?;
+                    let rev = term::args::oid(&val)?;
+
+                    revision_id = Some(rev);
+                }
+                Long("patch") | Short('p') if op == Some(OperationName::Review) => {
+                    if let review::Operation::Review { by_hunk, .. } = &mut review_op {
+                        *by_hunk = true;
+                    } else {
+                        return Err(arg.unexpected().into());
+                    }
+                }
+                Long("unified") | Short('U') if op == Some(OperationName::Review) => {
+                    if let review::Operation::Review { unified, .. } = &mut review_op {
+                        let val = parser.value()?;
+                        *unified = term::args::number(&val)?;
+                    } else {
+                        return Err(arg.unexpected().into());
+                    }
+                }
+                Long("hunk") if op == Some(OperationName::Review) => {
+                    if let review::Operation::Review { hunk, .. } = &mut review_op {
+                        let val = parser.value()?;
+                        let val = term::args::number(&val)
+                            .map_err(|e| anyhow!("invalid hunk value: {e}"))?;
+
+                        *hunk = Some(val);
+                    } else {
+                        return Err(arg.unexpected().into());
+                    }
+                }
+                Long("delete") | Short('d') if op == Some(OperationName::Review) => {
+                    review_op = review::Operation::Delete;
+                }
+                Long("accept") if op == Some(OperationName::Review) => {
+                    if let review::Operation::Review {
+                        verdict: verdict @ None,
+                        ..
+                    } = &mut review_op
+                    {
+                        *verdict = Some(patch::Verdict::Accept);
+                    } else {
+                        return Err(arg.unexpected().into());
+                    }
+                }
+                Long("reject") if op == Some(OperationName::Review) => {
+                    if let review::Operation::Review {
+                        verdict: verdict @ None,
+                        ..
+                    } = &mut review_op
+                    {
+                        *verdict = Some(patch::Verdict::Reject);
+                    } else {
+                        return Err(arg.unexpected().into());
+                    }
+                }
+
                 // Checkout options
                 Long("revision") if op == Some(OperationName::Checkout) => {
                     let val = parser.value()?;
@@ -409,6 +490,7 @@ impl Args for Options {
                     "assign" => op = Some(OperationName::Assign),
                     "label" => op = Some(OperationName::Label),
                     "comment" => op = Some(OperationName::Comment),
+                    "review" => op = Some(OperationName::Review),
                     "set" => op = Some(OperationName::Set),
                     unknown => anyhow::bail!("unknown operation '{}'", unknown),
                 },
@@ -426,6 +508,7 @@ impl Args for Options {
                             Some(OperationName::Ready),
                             Some(OperationName::Checkout),
                             Some(OperationName::Comment),
+                            Some(OperationName::Review),
                             Some(OperationName::Edit),
                             Some(OperationName::Set),
                             Some(OperationName::Assign),
@@ -467,6 +550,15 @@ impl Args for Options {
                 message,
                 reply_to,
             },
+            OperationName::Review => Operation::Review {
+                patch_id: patch_id
+                    .ok_or_else(|| anyhow!("a patch or revision must be provided"))?,
+                revision_id,
+                opts: review::Options {
+                    message,
+                    op: review_op,
+                },
+            },
             OperationName::Ready => Operation::Ready {
                 patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?,
                 undo,
@@ -585,6 +677,18 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
                 &profile,
             )?;
         }
+        Operation::Review {
+            patch_id,
+            revision_id,
+            opts,
+        } => {
+            let patch_id = patch_id.resolve(&repository.backend)?;
+            let revision_id = revision_id
+                .map(|rev| rev.resolve::<radicle::git::Oid>(&repository.backend))
+                .transpose()?
+                .map(patch::RevisionId::from);
+            review::run(patch_id, revision_id, opts, &profile, &repository)?;
+        }
         Operation::Edit { patch_id, message } => {
             let patch_id = patch_id.resolve(&repository.backend)?;
             edit::run(&patch_id, message, &profile, &repository)?;
diff --git a/radicle-cli/src/commands/patch/review.rs b/radicle-cli/src/commands/patch/review.rs
new file mode 100644
index 00000000..48d18856
--- /dev/null
+++ b/radicle-cli/src/commands/patch/review.rs
@@ -0,0 +1,141 @@
+#[path = "review/builder.rs"]
+mod builder;
+
+use anyhow::{anyhow, Context};
+
+use radicle::cob::patch::{PatchId, Patches, RevisionId, Verdict};
+use radicle::git;
+use radicle::prelude::*;
+use radicle::storage::git::Repository;
+
+use crate::terminal as term;
+use crate::terminal::patch::Message;
+
+/// Review help message.
+pub const REVIEW_HELP_MSG: &str = r#"
+<!--
+You may enter a review comment here. If you leave this blank,
+no comment will be attached to your review.
+
+Markdown supported.
+-->
+"#;
+
+#[derive(Debug, PartialEq, Eq)]
+pub enum Operation {
+    Delete,
+    Review {
+        by_hunk: bool,
+        unified: usize,
+        hunk: Option<usize>,
+        verdict: Option<Verdict>,
+    },
+}
+
+impl Default for Operation {
+    fn default() -> Self {
+        Self::Review {
+            by_hunk: false,
+            unified: 3,
+            hunk: None,
+            verdict: None,
+        }
+    }
+}
+
+#[derive(Debug, Default)]
+pub struct Options {
+    pub message: Message,
+    pub op: Operation,
+}
+
+pub fn run(
+    patch_id: PatchId,
+    revision_id: Option<RevisionId>,
+    options: Options,
+    profile: &Profile,
+    repository: &Repository,
+) -> anyhow::Result<()> {
+    let signer = term::signer(profile)?;
+    let _project = repository.identity_doc().context(format!(
+        "couldn't load project {} from local state",
+        repository.id
+    ))?;
+    let mut patches = Patches::open(repository)?;
+
+    let mut patch = patches
+        .get_mut(&patch_id)
+        .context(format!("couldn't find patch {patch_id} locally"))?;
+
+    let (revision_id, revision) = match revision_id {
+        Some(id) => (
+            id,
+            patch
+                .revision(&id)
+                .ok_or_else(|| anyhow!("Patch revision `{id}` not found"))?,
+        ),
+        None => patch.latest(),
+    };
+
+    let patch_id_pretty = term::format::tertiary(term::format::cob(&patch_id));
+    match options.op {
+        Operation::Review {
+            verdict,
+            by_hunk,
+            unified,
+            hunk,
+        } if by_hunk => {
+            let mut opts = git::raw::DiffOptions::new();
+            opts.patience(true)
+                .minimal(true)
+                .context_lines(unified as u32);
+
+            builder::ReviewBuilder::new(patch_id, *profile.id(), repository)
+                .hunk(hunk)
+                .verdict(verdict)
+                .run(revision, &mut opts)?;
+        }
+        Operation::Review { verdict, .. } => {
+            let message = options.message.get(REVIEW_HELP_MSG)?;
+            let message = message.replace(REVIEW_HELP_MSG.trim(), "");
+            let message = if message.is_empty() {
+                None
+            } else {
+                Some(message)
+            };
+            patch.review(revision_id, verdict, message, vec![], &signer)?;
+
+            match verdict {
+                Some(Verdict::Accept) => {
+                    term::success!(
+                        "Patch {} {}",
+                        patch_id_pretty,
+                        term::format::highlight("accepted")
+                    );
+                }
+                Some(Verdict::Reject) => {
+                    term::success!(
+                        "Patch {} {}",
+                        patch_id_pretty,
+                        term::format::negative("rejected")
+                    );
+                }
+                None => {
+                    term::success!("Patch {} reviewed", patch_id_pretty);
+                }
+            }
+        }
+        Operation::Delete => {
+            let name = git::refs::storage::draft::review(profile.id(), &patch_id);
+
+            match repository.backend.find_reference(&name) {
+                Ok(mut r) => r.delete()?,
+                Err(e) => {
+                    anyhow::bail!("Couldn't delete review reference '{name}': {e}");
+                }
+            }
+        }
+    }
+
+    Ok(())
+}
diff --git a/radicle-cli/src/commands/review/builder.rs b/radicle-cli/src/commands/patch/review/builder.rs
similarity index 100%
rename from radicle-cli/src/commands/review/builder.rs
rename to radicle-cli/src/commands/patch/review/builder.rs
diff --git a/radicle-cli/src/commands/review.rs b/radicle-cli/src/commands/review.rs
deleted file mode 100644
index c41c35ce..00000000
--- a/radicle-cli/src/commands/review.rs
+++ /dev/null
@@ -1,299 +0,0 @@
-#[path = "review/builder.rs"]
-mod builder;
-
-use std::ffi::OsString;
-
-use anyhow::{anyhow, Context};
-
-use radicle::cob::patch::{PatchId, Patches, RevisionId, Verdict};
-use radicle::prelude::*;
-use radicle::{git, rad};
-
-use crate::git::Rev;
-use crate::terminal as term;
-use crate::terminal::args::{string, Args, Error, Help};
-use crate::terminal::patch::Message;
-
-pub const HELP: Help = Help {
-    name: "review",
-    description: "Approve or reject a patch",
-    version: env!("CARGO_PKG_VERSION"),
-    usage: r#"
-Usage
-
-    rad review [<patch-id>] [--accept | --reject] [-m [<string>]] [<option>...]
-    rad review [<patch-id>] [-d | --delete]
-
-    To specify a patch to review, use the fully qualified patch id
-    or an unambiguous prefix of it.
-
-    In scripting contexts, patch mode can be used non-interactively,
-    by passing eg. the `--hunk` and `--accept` options.
-
-Options
-
-    -p, --patch               Review by patch hunks
-        --hunk <index>        Only review a specific hunk
-        --accept              Accept a patch or set of hunks
-        --reject              Reject a patch or set of hunks
-    -U, --unified <n>         Generate diffs with <n> lines of context instead of the usual three
-    -d, --delete              Delete a review draft
-    -r, --revision            Review a patch revision
-        --[no-]sync           Sync review to seed (default: sync)
-    -m, --message [<string>]  Provide a comment with the review (default: prompt)
-        --help                Print help
-"#,
-};
-
-/// Review help message.
-pub const REVIEW_HELP_MSG: &str = r#"
-<!--
-You may enter a review comment here. If you leave this blank,
-no comment will be attached to your review.
-
-Markdown supported.
--->
-"#;
-
-#[derive(Debug, PartialEq, Eq)]
-pub enum Operation {
-    Delete,
-    Review {
-        by_hunk: bool,
-        unified: usize,
-        hunk: Option<usize>,
-        verdict: Option<Verdict>,
-    },
-}
-
-impl Default for Operation {
-    fn default() -> Self {
-        Self::Review {
-            by_hunk: false,
-            unified: 3,
-            hunk: None,
-            verdict: None,
-        }
-    }
-}
-
-#[derive(Debug)]
-pub struct Options {
-    pub id: Rev,
-    pub revision: bool,
-    pub message: Message,
-    pub sync: bool,
-    pub verbose: bool,
-    pub op: Operation,
-}
-
-impl Args for Options {
-    fn from_args(args: Vec<OsString>) -> anyhow::Result<(Self, Vec<OsString>)> {
-        use lexopt::prelude::*;
-
-        let mut parser = lexopt::Parser::from_args(args);
-        let mut id: Option<Rev> = None;
-        let mut revision = false;
-        let mut message = Message::default();
-        let mut sync = true;
-        let mut verbose = false;
-        let mut op = Operation::default();
-
-        while let Some(arg) = parser.next()? {
-            match arg {
-                Long("help") | Short('h') => {
-                    return Err(Error::Help.into());
-                }
-                Long("revision") | Short('r') => {
-                    revision = true;
-                }
-                Long("sync") => {
-                    // Skipping due the `no-sync` flag precedence.
-                }
-                Long("no-sync") => {
-                    sync = false;
-                }
-                Long("message") | Short('m') => {
-                    if message != Message::Blank {
-                        let txt: String = parser.value()?.to_string_lossy().into();
-                        message.append(&txt);
-                    }
-                }
-                Long("no-message") => {
-                    message = Message::Blank;
-                }
-                Long("patch") | Short('p') => {
-                    if let Operation::Review { by_hunk, .. } = &mut op {
-                        *by_hunk = true;
-                    } else {
-                        return Err(arg.unexpected().into());
-                    }
-                }
-                Long("unified") | Short('U') => {
-                    if let Operation::Review { unified, .. } = &mut op {
-                        let val = parser.value()?;
-                        *unified = term::args::number(&val)?;
-                    } else {
-                        return Err(arg.unexpected().into());
-                    }
-                }
-                Long("hunk") => {
-                    if let Operation::Review { hunk, .. } = &mut op {
-                        let val = parser.value()?;
-                        let val = term::args::number(&val)
-                            .map_err(|e| anyhow!("invalid hunk value: {e}"))?;
-
-                        *hunk = Some(val);
-                    } else {
-                        return Err(arg.unexpected().into());
-                    }
-                }
-                Long("delete") | Short('d') => {
-                    op = Operation::Delete;
-                }
-                Long("verbose") | Short('v') => {
-                    verbose = true;
-                }
-                Long("accept") => {
-                    if let Operation::Review {
-                        verdict: verdict @ None,
-                        ..
-                    } = &mut op
-                    {
-                        *verdict = Some(Verdict::Accept);
-                    } else {
-                        return Err(arg.unexpected().into());
-                    }
-                }
-                Long("reject") => {
-                    if let Operation::Review {
-                        verdict: verdict @ None,
-                        ..
-                    } = &mut op
-                    {
-                        *verdict = Some(Verdict::Reject);
-                    } else {
-                        return Err(arg.unexpected().into());
-                    }
-                }
-                Value(val) => {
-                    id = Some(Rev::from(string(&val)));
-                }
-                _ => return Err(anyhow::anyhow!(arg.unexpected())),
-            }
-        }
-
-        Ok((
-            Options {
-                id: id.ok_or_else(|| anyhow!("a patch to review must be provided"))?,
-                message,
-                sync,
-                revision,
-                verbose,
-                op,
-            },
-            vec![],
-        ))
-    }
-}
-
-pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
-    let (_, id) =
-        rad::cwd().map_err(|_| anyhow!("this command must be run in the context of a project"))?;
-    let profile = ctx.profile()?;
-    let signer = term::signer(&profile)?;
-    let repository = profile.storage.repository(id)?;
-    let _project = repository
-        .identity_doc()
-        .context(format!("couldn't load project {id} from local state"))?;
-    let mut patches = Patches::open(&repository)?;
-
-    let (patch_id, revision) = if options.revision {
-        let id = options.id.resolve::<git::Oid>(&repository.backend)?;
-        let (patch_id, _, rev_id, rev) = patches
-            .find_by_revision(&RevisionId::from(id))?
-            .ok_or_else(|| anyhow!("revision {} does not exist", id))?;
-
-        (patch_id, Some((rev_id, rev)))
-    } else {
-        let id = options.id.resolve::<PatchId>(&repository.backend)?;
-        (id, None)
-    };
-
-    let mut patch = patches
-        .get_mut(&patch_id)
-        .context(format!("couldn't find patch {patch_id} locally"))?;
-
-    let (revision_id, revision) = if let Some(v) = revision {
-        v
-    } else {
-        let (id, r) = patch.latest();
-        (id, r.clone())
-    };
-
-    let patch_id_pretty = term::format::tertiary(term::format::cob(&patch_id));
-    match options.op {
-        Operation::Review {
-            verdict,
-            by_hunk,
-            unified,
-            hunk,
-        } if by_hunk => {
-            let mut opts = git::raw::DiffOptions::new();
-            opts.patience(true)
-                .minimal(true)
-                .context_lines(unified as u32);
-
-            builder::ReviewBuilder::new(patch_id, *profile.id(), &repository)
-                .hunk(hunk)
-                .verdict(verdict)
-                .run(&revision, &mut opts)?;
-        }
-        Operation::Review { verdict, .. } => {
-            let message = options.message.get(REVIEW_HELP_MSG)?;
-            let message = message.replace(REVIEW_HELP_MSG.trim(), "");
-            let message = if message.is_empty() {
-                None
-            } else {
-                Some(message)
-            };
-            patch.review(revision_id, verdict, message, vec![], &signer)?;
-
-            match verdict {
-                Some(Verdict::Accept) => {
-                    term::success!(
-                        "Patch {} {}",
-                        patch_id_pretty,
-                        term::format::highlight("accepted")
-                    );
-                }
-                Some(Verdict::Reject) => {
-                    term::success!(
-                        "Patch {} {}",
-                        patch_id_pretty,
-                        term::format::negative("rejected")
-                    );
-                }
-                None => {
-                    term::success!("Patch {} reviewed", patch_id_pretty);
-                }
-            }
-        }
-        Operation::Delete => {
-            let name = git::refs::storage::draft::review(profile.id(), &patch_id);
-
-            match repository.backend.find_reference(&name) {
-                Ok(mut r) => r.delete()?,
-                Err(e) => {
-                    anyhow::bail!("Couldn't delete review reference '{name}': {e}");
-                }
-            }
-        }
-    }
-
-    if options.sync {
-        term::warning("the `--sync` option is not yet supported");
-    }
-
-    Ok(())
-}
diff --git a/radicle-cli/src/main.rs b/radicle-cli/src/main.rs
index 32cbf74f..a5ee2217 100644
--- a/radicle-cli/src/main.rs
+++ b/radicle-cli/src/main.rs
@@ -208,13 +208,6 @@ fn run_other(exe: &str, args: &[OsString]) -> Result<(), Option<anyhow::Error>>
                 args.to_vec(),
             );
         }
-        "review" => {
-            term::run_command_args::<rad_review::Options, _>(
-                rad_review::HELP,
-                rad_review::run,
-                args.to_vec(),
-            );
-        }
         "clean" => {
             term::run_command_args::<rad_clean::Options, _>(
                 rad_clean::HELP,