Skip to content

Commit

Permalink
feat!: move prs create>send, prs list>list
Browse files Browse the repository at this point in the history
remove unnecessary hierachy of `prs` which is also a troublesome term

replace the concept of `create` which aligns more to the PR github model
to `send` which aligns more with the git patch model
  • Loading branch information
DanConwayDev committed Feb 14, 2024
1 parent 1022344 commit 6f2ec87
Show file tree
Hide file tree
Showing 12 changed files with 43 additions and 57 deletions.
6 changes: 3 additions & 3 deletions src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use anyhow::{bail, Context, Result};
use git2::{DiffOptions, Oid, Revwalk};
use nostr::prelude::{sha1::Hash as Sha1Hash, Hash};

use crate::sub_commands::prs::list::tag_value;
use crate::sub_commands::list::tag_value;

pub struct Repo {
git_repo: git2::Repository,
Expand Down Expand Up @@ -1251,7 +1251,7 @@ mod tests {
mod apply_patch {

use super::*;
use crate::{repo_ref::RepoRef, sub_commands::prs::create::generate_patch_event};
use crate::{repo_ref::RepoRef, sub_commands::send::generate_patch_event};

fn generate_patch_from_head_commit(test_repo: &GitTestRepo) -> Result<nostr::Event> {
let original_oid = test_repo.git_repo.head()?.peel_to_commit()?.id();
Expand Down Expand Up @@ -1405,7 +1405,7 @@ mod tests {
use test_utils::TEST_KEY_1_KEYS;

use super::*;
use crate::{repo_ref::RepoRef, sub_commands::prs::create::generate_pr_and_patch_events};
use crate::{repo_ref::RepoRef, sub_commands::send::generate_pr_and_patch_events};

static BRANCH_NAME: &str = "add-example-feature";
// returns original_repo, pr_event, patch_events
Expand Down
9 changes: 6 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ enum Commands {
Login(sub_commands::login::SubCommandArgs),
/// issue repository reference event as a maintainers
Claim(sub_commands::claim::SubCommandArgs),
/// create and issue prs
Prs(sub_commands::prs::SubCommandArgs),
/// send a PR / patch / patch set via nostr events
Send(sub_commands::send::SubCommandArgs),
/// list open PRs / patches / patch sets and pull / apply them a branch
List(sub_commands::list::SubCommandArgs),
/// pull latest commits in pr linked to checked out branch
Pull,
/// push commits to current checked out pr branch
Expand All @@ -50,7 +52,8 @@ async fn main() -> Result<()> {
match &cli.command {
Commands::Login(args) => sub_commands::login::launch(&cli, args).await,
Commands::Claim(args) => sub_commands::claim::launch(&cli, args).await,
Commands::Prs(args) => sub_commands::prs::launch(&cli, args).await,
Commands::Send(args) => sub_commands::send::launch(&cli, args).await,
Commands::List(args) => sub_commands::list::launch(&cli, args).await,
Commands::Pull => sub_commands::pull::launch().await,
Commands::Push => sub_commands::push::launch(&cli).await,
}
Expand Down
2 changes: 1 addition & 1 deletion src/sub_commands/claim.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use anyhow::{Context, Result};

use super::prs::create::send_events;
use super::send::send_events;
#[cfg(not(test))]
use crate::client::Client;
#[cfg(test)]
Expand Down
10 changes: 3 additions & 7 deletions src/sub_commands/prs/list.rs → src/sub_commands/list.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use anyhow::{bail, Context, Result};

use super::create::event_is_patch_set_root;
use super::send::event_is_patch_set_root;
#[cfg(not(test))]
use crate::client::Client;
#[cfg(test)]
Expand All @@ -10,7 +10,7 @@ use crate::{
client::Connect,
git::{Repo, RepoActions},
repo_ref::{self, RepoRef, REPO_REF_KIND},
sub_commands::prs::create::{event_is_cover_letter, event_to_cover_letter, PATCH_KIND},
sub_commands::send::{event_is_cover_letter, event_to_cover_letter, PATCH_KIND},
Cli,
};

Expand All @@ -22,11 +22,7 @@ pub struct SubCommandArgs {
}

#[allow(clippy::too_many_lines)]
pub async fn launch(
_cli_args: &Cli,
_pr_args: &super::SubCommandArgs,
_args: &SubCommandArgs,
) -> Result<()> {
pub async fn launch(_cli_args: &Cli, _args: &SubCommandArgs) -> Result<()> {
let git_repo = Repo::discover().context("cannot find a git repository")?;

let root_commit = git_repo
Expand Down
3 changes: 2 additions & 1 deletion src/sub_commands/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod claim;
pub mod list;
pub mod login;
pub mod prs;
pub mod pull;
pub mod push;
pub mod send;
2 changes: 1 addition & 1 deletion src/sub_commands/pull.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
git::{Repo, RepoActions},
repo_ref,
sub_commands::{
prs::list::get_most_recent_patch_with_ancestors, push::fetch_pr_and_most_recent_patch_chain,
list::get_most_recent_patch_with_ancestors, push::fetch_pr_and_most_recent_patch_chain,
},
};

Expand Down
4 changes: 2 additions & 2 deletions src/sub_commands/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ use crate::{
git::{str_to_sha1, Repo, RepoActions},
login,
repo_ref::{self, RepoRef},
sub_commands::prs::{
create::{event_to_cover_letter, generate_patch_event, send_events},
sub_commands::{
list::{
find_commits_for_pr_event, find_pr_events, get_most_recent_patch_with_ancestors,
tag_value,
},
send::{event_to_cover_letter, generate_patch_event, send_events},
},
Cli,
};
Expand Down
7 changes: 2 additions & 5 deletions src/sub_commands/prs/create.rs → src/sub_commands/send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,7 @@ pub struct SubCommandArgs {
}

#[allow(clippy::too_many_lines)]
pub async fn launch(
cli_args: &Cli,
_pr_args: &super::SubCommandArgs,
args: &SubCommandArgs,
) -> Result<()> {
pub async fn launch(cli_args: &Cli, args: &SubCommandArgs) -> Result<()> {
let git_repo = Repo::discover().context("cannot find a git repository")?;

let (from_branch, to_branch, mut ahead, behind) =
Expand Down Expand Up @@ -178,6 +174,7 @@ pub async fn launch(
Ok(())
}

#[allow(clippy::module_name_repetitions)]
pub async fn send_events(
#[cfg(test)] client: &crate::client::MockConnect,
#[cfg(not(test))] client: &Client,
Expand Down
26 changes: 12 additions & 14 deletions tests/prs_list.rs → tests/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ fn cli_tester_create_pr(
"--password",
TEST_PASSWORD,
"--disable-cli-spinners",
"prs",
"create",
"send",
"--title",
format!("\"{title}\"").as_str(),
"--description",
Expand All @@ -95,8 +94,7 @@ fn cli_tester_create_pr(
"--password",
TEST_PASSWORD,
"--disable-cli-spinners",
"prs",
"create",
"send",
"--no-cover-letter",
],
);
Expand Down Expand Up @@ -143,7 +141,7 @@ mod when_main_branch_is_uptodate {

let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

p.expect("finding PRs...\r\n")?;
let mut c = p.expect_choice(
Expand Down Expand Up @@ -203,7 +201,7 @@ mod when_main_branch_is_uptodate {

let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

p.expect("finding PRs...\r\n")?;
let mut c = p.expect_choice(
Expand Down Expand Up @@ -308,7 +306,7 @@ mod when_main_branch_is_uptodate {

let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

p.expect("finding PRs...\r\n")?;
let mut c = p.expect_choice(
Expand Down Expand Up @@ -368,7 +366,7 @@ mod when_main_branch_is_uptodate {

let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

p.expect("finding PRs...\r\n")?;
let mut c = p.expect_choice(
Expand Down Expand Up @@ -478,7 +476,7 @@ mod when_main_branch_is_uptodate {
)?;
let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

p.expect("finding PRs...\r\n")?;
let mut c = p.expect_choice(
Expand Down Expand Up @@ -544,7 +542,7 @@ mod when_main_branch_is_uptodate {
)?;
let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

p.expect("finding PRs...\r\n")?;
let mut c = p.expect_choice(
Expand Down Expand Up @@ -658,7 +656,7 @@ mod when_main_branch_is_uptodate {

let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

create_and_populate_branch(
&test_repo,
Expand Down Expand Up @@ -725,7 +723,7 @@ mod when_main_branch_is_uptodate {

let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

create_and_populate_branch(
&test_repo,
Expand Down Expand Up @@ -817,7 +815,7 @@ mod when_main_branch_is_uptodate {

let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

create_and_populate_branch(
&test_repo,
Expand Down Expand Up @@ -885,7 +883,7 @@ mod when_main_branch_is_uptodate {

let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "list"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["list"]);

create_and_populate_branch(
&test_repo,
Expand Down
3 changes: 1 addition & 2 deletions tests/pull.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ fn cli_tester_create_pr(
"--password",
TEST_PASSWORD,
"--disable-cli-spinners",
"prs",
"create",
"send",
"--title",
format!("\"{title}\"").as_str(),
"--description",
Expand Down
3 changes: 1 addition & 2 deletions tests/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ fn cli_tester_create_pr(
"--password",
TEST_PASSWORD,
"--disable-cli-spinners",
"prs",
"create",
"send",
"--title",
format!("\"{title}\"").as_str(),
"--description",
Expand Down
25 changes: 9 additions & 16 deletions tests/prs_create.rs → tests/send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ use test_utils::{git::GitTestRepo, relay::Relay, *};
fn when_to_branch_doesnt_exist_return_error() -> Result<()> {
let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(
&test_repo.dir,
["prs", "create", "--to-branch", "nonexistant"],
);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "--to-branch", "nonexistant"]);
p.expect("Error: cannot find to_branch 'nonexistant'")?;
Ok(())
}
Expand All @@ -19,7 +16,7 @@ fn when_to_branch_doesnt_exist_return_error() -> Result<()> {
fn when_no_to_branch_specified_and_no_main_or_master_branch_return_error() -> Result<()> {
let test_repo = GitTestRepo::new("notmain")?;
test_repo.populate()?;
let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "create"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);
p.expect("Error: a destination branch (to_branch) is not specified and the defaults (main or master) do not exist")?;
Ok(())
}
Expand All @@ -28,10 +25,7 @@ fn when_no_to_branch_specified_and_no_main_or_master_branch_return_error() -> Re
fn when_from_branch_doesnt_exist_return_error() -> Result<()> {
let test_repo = GitTestRepo::default();
test_repo.populate()?;
let mut p = CliTester::new_from_dir(
&test_repo.dir,
["prs", "create", "--from-branch", "nonexistant"],
);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "--from-branch", "nonexistant"]);
p.expect("Error: cannot find from_branch 'nonexistant'")?;
Ok(())
}
Expand All @@ -44,7 +38,7 @@ fn when_no_commits_ahead_of_main_return_error() -> Result<()> {
test_repo.create_branch("feature")?;
test_repo.checkout("feature")?;

let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "create"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);
p.expect("Error: 'head' is 0 commits ahead of 'main' so no patches were created")?;
Ok(())
}
Expand Down Expand Up @@ -88,7 +82,7 @@ mod when_commits_behind_ask_to_proceed {
fn asked_with_default_no() -> Result<()> {
let test_repo = prep_test_repo()?;

let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "create"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);
expect_confirm_prompt(&mut p, BEHIND_LEN, AHEAD_LEN)?;
p.exit()?;
Ok(())
Expand All @@ -98,7 +92,7 @@ mod when_commits_behind_ask_to_proceed {
fn when_response_is_false_aborts() -> Result<()> {
let test_repo = prep_test_repo()?;

let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "create"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);

expect_confirm_prompt(&mut p, BEHIND_LEN, AHEAD_LEN)?.succeeds_with(Some(false))?;

Expand All @@ -111,7 +105,7 @@ mod when_commits_behind_ask_to_proceed {
fn when_response_is_true_proceeds() -> Result<()> {
let test_repo = prep_test_repo()?;

let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "create"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);
expect_confirm_prompt(&mut p, BEHIND_LEN, AHEAD_LEN)?.succeeds_with(Some(true))?;
p.expect(
format!("creating patch for {AHEAD_LEN} commits from 'head' that are {BEHIND_LEN} behind 'main'",)
Expand All @@ -135,7 +129,7 @@ fn cli_message_creating_patches() -> Result<()> {
std::fs::write(test_repo.dir.join("t4.md"), "some content")?;
test_repo.stage_and_commit("add t4.md")?;

let mut p = CliTester::new_from_dir(&test_repo.dir, ["prs", "create"]);
let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);

p.expect("creating patch for 2 commits from 'head' that can be merged into 'main'")?;
p.exit()?;
Expand Down Expand Up @@ -172,8 +166,7 @@ fn cli_tester_create_pr(git_repo: &GitTestRepo, include_cover_letter: bool) -> C
"--password",
TEST_PASSWORD,
"--disable-cli-spinners",
"prs",
"create",
"send",
];
if include_cover_letter {
for arg in [
Expand Down

0 comments on commit 6f2ec87

Please sign in to comment.