From 701668b02d999af42f51d8bd25fffb2a8692c3c8 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 16 Feb 2024 22:31:29 +0000 Subject: refactor: rename PR to proposal PR is a problematic term when it ambiguous whether the set of patches are PR-like or email-patch like. --- src/git.rs | 24 +++++++++++--------- src/main.rs | 2 +- src/sub_commands/list.rs | 54 +++++++++++++++++++++++--------------------- src/sub_commands/pull.rs | 21 ++++++++++------- src/sub_commands/push.rs | 59 ++++++++++++++++++++++++++---------------------- src/sub_commands/send.rs | 4 ++-- 6 files changed, 89 insertions(+), 75 deletions(-) (limited to 'src') diff --git a/src/git.rs b/src/git.rs index 13d56f3..faeb9f9 100644 --- a/src/git.rs +++ b/src/git.rs @@ -372,7 +372,7 @@ impl RepoActions for Repo { if let Ok(last_patch) = patches_to_apply.last().context("no patches") { last_patch } else { - self.checkout(branch_name).context("latest commit in pr doesnt connect with an existing commit. Try a git pull on master first.")?; + self.checkout(branch_name).context("latest commit in proposal doesnt connect with an existing commit. Try a git pull on master first.")?; return Ok(vec![]); }, "parent-commit", @@ -387,9 +387,9 @@ impl RepoActions for Repo { if let Ok(current_tip) = self.get_tip_of_local_branch(branch_name) { if !current_tip.to_string().eq(&parent_commit_id) { // TODO: either changes have been made on the local branch or - // the latest commit in the pr has rebased onto a newer commit - // that you havn't pulled yet ask user whether - // they want to proceed + // the latest commit in the prpoosal has rebased onto a newer + // commit that you havn't pulled yet ask user + // whether they want to proceed } } @@ -1408,20 +1408,22 @@ mod tests { use test_utils::TEST_KEY_1_KEYS; use super::*; - use crate::{repo_ref::RepoRef, sub_commands::send::generate_pr_and_patch_events}; + use crate::{ + repo_ref::RepoRef, sub_commands::send::generate_cover_letter_and_patch_events, + }; static BRANCH_NAME: &str = "add-example-feature"; - // returns original_repo, pr_event, patch_events + // returns original_repo, cover_letter_event, patch_events fn generate_test_repo_and_events() -> Result<(GitTestRepo, nostr::Event, Vec)> { let original_repo = GitTestRepo::default(); let oid3 = original_repo.populate_with_test_branch()?; let oid2 = original_repo.git_repo.find_commit(oid3)?.parent_id(0)?; let oid1 = original_repo.git_repo.find_commit(oid2)?.parent_id(0)?; - // TODO: generate pr and patch events + // TODO: generate cover_letter and patch events let git_repo = Repo::from_path(&original_repo.dir)?; - let mut events = generate_pr_and_patch_events( + let mut events = generate_cover_letter_and_patch_events( Some(("test".to_string(), "test".to_string())), &git_repo, &vec![oid_to_sha1(&oid1), oid_to_sha1(&oid2), oid_to_sha1(&oid3)], @@ -1441,7 +1443,7 @@ mod tests { use super::*; #[test] - fn branch_gets_created_with_name_specified_in_pr() -> Result<()> { + fn branch_gets_created_with_name_specified_in_proposal() -> Result<()> { let (_, _, patch_events) = generate_test_repo_and_events()?; let test_repo = GitTestRepo::default(); test_repo.populate()?; @@ -1530,7 +1532,7 @@ mod tests { use super::*; #[test] - fn branch_gets_created_with_name_specified_in_pr() -> Result<()> { + fn branch_gets_created_with_name_specified_in_proposal() -> Result<()> { let (_, _, patch_events) = generate_test_repo_and_events()?; let test_repo = GitTestRepo::default(); test_repo.populate()?; @@ -1609,7 +1611,7 @@ mod tests { } } - // TODO when_pr_root_is_tip_ahead_of_main_and_doesnt_exist + // TODO when_proposal_root_is_tip_ahead_of_main_and_doesnt_exist } mod when_branch_and_first_commits_exists { diff --git a/src/main.rs b/src/main.rs index 82a87c8..f60edd8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -41,7 +41,7 @@ enum Commands { List(sub_commands::list::SubCommandArgs), /// send new commits as proposal ammendments Push, - /// pull latest commits in pr linked to checked out branch + /// pull latest commits in proposal linked to checked out branch Pull, /// run with --nsec flag to change npub Login(sub_commands::login::SubCommandArgs), diff --git a/src/sub_commands/list.rs b/src/sub_commands/list.rs index 4764adc..b8c2919 100644 --- a/src/sub_commands/list.rs +++ b/src/sub_commands/list.rs @@ -46,21 +46,21 @@ pub async fn launch(_cli_args: &Cli, _args: &SubCommandArgs) -> Result<()> { ) .await?; - println!("finding PRs..."); + println!("finding proposals..."); - let pr_events: Vec = - find_pr_events(&client, &repo_ref, &root_commit.to_string()).await?; + let proposal_events: Vec = + find_proposal_events(&client, &repo_ref, &root_commit.to_string()).await?; - if pr_events.is_empty() { - println!("no PRs found... create one? try `ngit send`"); + if proposal_events.is_empty() { + println!("no proposals found... create one? try `ngit send`"); return Ok(()); } let selected_index = Interactor::default().choice( PromptChoiceParms::default() - .with_prompt("All PRs") + .with_prompt("all proposals") .with_choices( - pr_events + proposal_events .iter() .map(|e| { if let Ok(cl) = event_to_cover_letter(e) { @@ -78,26 +78,27 @@ pub async fn launch(_cli_args: &Cli, _args: &SubCommandArgs) -> Result<()> { println!("finding commits..."); let commits_events: Vec = - find_commits_for_pr_event(&client, &pr_events[selected_index], &repo_ref).await?; + find_commits_for_proposal_root_event(&client, &proposal_events[selected_index], &repo_ref) + .await?; confirm_checkout(&git_repo)?; - let most_recent_pr_patch_chain = get_most_recent_patch_with_ancestors(commits_events) - .context("cannot get most recent patch for PR")?; + let most_recent_proposal_patch_chain = get_most_recent_patch_with_ancestors(commits_events) + .context("cannot get most recent patch for proposal")?; - let branch_name: String = event_to_cover_letter(&pr_events[selected_index]) + let branch_name: String = event_to_cover_letter(&proposal_events[selected_index]) .context("cannot assign a branch name as event is not a patch set root")? .branch_name; let applied = git_repo - .apply_patch_chain(&branch_name, most_recent_pr_patch_chain) + .apply_patch_chain(&branch_name, most_recent_proposal_patch_chain) .context("cannot apply patch chain")?; if applied.is_empty() { - println!("checked out PR branch. no new commits to pull"); + println!("checked out proposal branch. no new commits to pull"); } else { println!( - "checked out PR branch. pulled {} new commits", + "checked out proposal branch. pulled {} new commits", applied.len(), ); } @@ -115,7 +116,7 @@ fn confirm_checkout(git_repo: &Repo) -> Result<()> { if git_repo.has_outstanding_changes()? { bail!( - "cannot pull PR branch when repository is not clean. discard or stash (un)staged changes and try again." + "cannot pull proposal branch when repository is not clean. discard or stash (un)staged changes and try again." ); } Ok(()) @@ -201,7 +202,7 @@ pub fn get_most_recent_patch_with_ancestors( Ok(res) } -pub async fn find_pr_events( +pub async fn find_proposal_events( #[cfg(test)] client: &crate::client::MockConnect, #[cfg(not(test))] client: &Client, repo_ref: &RepoRef, @@ -221,7 +222,8 @@ pub async fn find_pr_events( .iter() .map(|m| format!("{REPO_REF_KIND}:{m}:{}", repo_ref.identifier)), ), - // also pick up prs from the same repo but no target at our maintainers repo events + // also pick up proposals from the same repo but no target at our maintainers repo + // events nostr::Filter::default() .kind(nostr::Kind::Custom(PATCH_KIND)) .custom_tag(nostr::Alphabet::T, vec!["root"]) @@ -229,7 +231,7 @@ pub async fn find_pr_events( ], ) .await - .context("cannot get pr events")? + .context("cannot get proposal events")? .iter() .filter(|e| { event_is_patch_set_root(e) @@ -250,10 +252,10 @@ pub async fn find_pr_events( .collect::>()) } -pub async fn find_commits_for_pr_event( +pub async fn find_commits_for_proposal_root_event( #[cfg(test)] client: &crate::client::MockConnect, #[cfg(not(test))] client: &Client, - pr_event: &nostr::Event, + proposal_root_event: &nostr::Event, repo_ref: &RepoRef, ) -> Result> { let mut patch_events: Vec = client @@ -265,7 +267,7 @@ pub async fn find_commits_for_pr_event( // this requires every patch to reference the root event // this will not pick up v2,v3 patch sets // TODO: fetch commits for v2.. patch sets - .event(pr_event.id), + .event(proposal_root_event.id), ], ) .await @@ -273,15 +275,15 @@ pub async fn find_commits_for_pr_event( .iter() .filter(|e| { e.kind.as_u64() == PATCH_KIND - && e.tags - .iter() - .any(|t| t.as_vec().len() > 2 && t.as_vec()[1].eq(&pr_event.id.to_string())) + && e.tags.iter().any(|t| { + t.as_vec().len() > 2 && t.as_vec()[1].eq(&proposal_root_event.id.to_string()) + }) }) .map(std::borrow::ToOwned::to_owned) .collect(); - if !event_is_cover_letter(pr_event) { - patch_events.push(pr_event.clone()); + if !event_is_cover_letter(proposal_root_event) { + patch_events.push(proposal_root_event.clone()); } Ok(patch_events) } diff --git a/src/sub_commands/pull.rs b/src/sub_commands/pull.rs index fc6db37..de078e3 100644 --- a/src/sub_commands/pull.rs +++ b/src/sub_commands/pull.rs @@ -9,7 +9,8 @@ use crate::{ git::{Repo, RepoActions}, repo_ref, sub_commands::{ - list::get_most_recent_patch_with_ancestors, push::fetch_pr_and_most_recent_patch_chain, + list::get_most_recent_patch_with_ancestors, + push::fetch_proposal_root_and_most_recent_patch_chain, }, }; @@ -29,7 +30,7 @@ pub async fn launch() -> Result<()> { .context("cannot get checked out branch name")?; if branch_name == main_or_master_branch_name { - bail!("checkout a branch associated with a PR first") + bail!("checkout a branch associated with a proposal first") } #[cfg(not(test))] let client = Client::default(); @@ -44,19 +45,23 @@ pub async fn launch() -> Result<()> { ) .await?; - let (_pr_event, commit_events) = - fetch_pr_and_most_recent_patch_chain(&client, &repo_ref, &root_commit, &branch_name) - .await?; + let (_proposal_root_event, commit_events) = fetch_proposal_root_and_most_recent_patch_chain( + &client, + &repo_ref, + &root_commit, + &branch_name, + ) + .await?; if git_repo.has_outstanding_changes()? { bail!("cannot pull changes when repository is not clean. discard changes and try again."); } - let most_recent_pr_patch_chain = get_most_recent_patch_with_ancestors(commit_events) - .context("cannot get most recent patch for PR")?; + let most_recent_proposal_patch_chain = get_most_recent_patch_with_ancestors(commit_events) + .context("cannot get most recent patch for proposal")?; let applied = git_repo - .apply_patch_chain(&branch_name, most_recent_pr_patch_chain) + .apply_patch_chain(&branch_name, most_recent_proposal_patch_chain) .context("cannot apply patch chain")?; if applied.is_empty() { diff --git a/src/sub_commands/push.rs b/src/sub_commands/push.rs index 7c6b95b..0fdd56f 100644 --- a/src/sub_commands/push.rs +++ b/src/sub_commands/push.rs @@ -12,7 +12,7 @@ use crate::{ repo_ref::{self, RepoRef}, sub_commands::{ list::{ - find_commits_for_pr_event, find_pr_events, get_commit_id_from_patch, + find_commits_for_proposal_root_event, find_proposal_events, get_commit_id_from_patch, get_most_recent_patch_with_ancestors, tag_value, }, send::{event_to_cover_letter, generate_patch_event, send_events}, @@ -36,7 +36,7 @@ pub async fn launch(cli_args: &Cli) -> Result<()> { .context("cannot get checked out branch name")?; if branch_name == main_or_master_branch_name { - bail!("checkout a branch associated with a PR first") + bail!("checkout a branch associated with a proposal first") } #[cfg(not(test))] let mut client = Client::default(); @@ -51,44 +51,48 @@ pub async fn launch(cli_args: &Cli) -> Result<()> { ) .await?; - let (pr_event, commit_events) = - fetch_pr_and_most_recent_patch_chain(&client, &repo_ref, &root_commit, &branch_name) - .await?; + let (proposal_root_event, commit_events) = fetch_proposal_root_and_most_recent_patch_chain( + &client, + &repo_ref, + &root_commit, + &branch_name, + ) + .await?; // TODO: fix these scenarios: - // - local PR branch is 2 behind and 1 ahead. intructions: ... - // - PR has been rebased. (against commit in main) instructions: ... - // - PR has been rebased. (against commit not in repo) instructions: .. + // - local proposal branch is 2 behind and 1 ahead. intructions: ... + // - proposal has been rebased. (against commit in main) instructions: ... + // - proposal has been rebased. (against commit not in repo) instructions: .. - let most_recent_pr_patch_chain = get_most_recent_patch_with_ancestors(commit_events) - .context("cannot get most recent patch for PR")?; + let most_recent_proposal_patch_chain = get_most_recent_patch_with_ancestors(commit_events) + .context("cannot get most recent patch for proposal")?; let branch_tip = git_repo.get_tip_of_local_branch(&branch_name)?; let most_recent_patch_commit_id = str_to_sha1( - &get_commit_id_from_patch(&most_recent_pr_patch_chain[0]) + &get_commit_id_from_patch(&most_recent_proposal_patch_chain[0]) .context("latest patch event doesnt have a commit tag")?, ) .context("latest patch event commit tag isn't a valid SHA1 hash")?; if most_recent_patch_commit_id.eq(&branch_tip) { - bail!("nostr pr already up-to-date with local branch"); + bail!("nostr proposal already up-to-date with local branch"); } - if most_recent_pr_patch_chain.iter().any(|e| { + if most_recent_proposal_patch_chain.iter().any(|e| { let c = tag_value(e, "parent-commit").unwrap_or_default(); c.eq(&branch_tip.to_string()) }) { - bail!("nostr pr is ahead of local branch"); + bail!("nostr proposal is ahead of local branch"); } let (ahead, behind) = git_repo .get_commits_ahead_behind(&most_recent_patch_commit_id, &branch_tip) - .context("the latest patch in pr doesnt share an ancestor with your branch.")?; + .context("the latest patch in proposal doesnt share an ancestor with your branch.")?; if !behind.is_empty() { bail!( - "your local pr branch is {} behind patches on nostr. consider rebasing or force pushing", + "your local proposal branch is {} behind patches on nostr. consider rebasing or force pushing", behind.len() ) } @@ -109,7 +113,7 @@ pub async fn launch(cli_args: &Cli) -> Result<()> { &git_repo, &root_commit, commit, - Some(pr_event.id), + Some(proposal_root_event.id), &keys, &repo_ref, patch_events.last().map(nostr::Event::id), @@ -135,33 +139,34 @@ pub async fn launch(cli_args: &Cli) -> Result<()> { Ok(()) } -pub async fn fetch_pr_and_most_recent_patch_chain( +pub async fn fetch_proposal_root_and_most_recent_patch_chain( #[cfg(test)] client: &crate::client::MockConnect, #[cfg(not(test))] client: &Client, repo_ref: &RepoRef, root_commit: &Sha1Hash, branch_name: &String, ) -> Result<(nostr::Event, Vec)> { - println!("finding PR event..."); + println!("finding proposal root event..."); - let pr_events: Vec = find_pr_events(client, repo_ref, &root_commit.to_string()) - .await - .context("cannot get pr events for repo")?; + let proposal_events: Vec = + find_proposal_events(client, repo_ref, &root_commit.to_string()) + .await + .context("cannot get proposal events for repo")?; - let pr_event: nostr::Event = pr_events + let proposal_root_event: nostr::Event = proposal_events .iter() .find(|e| { event_to_cover_letter(e).is_ok_and(|cl| cl.branch_name.eq(branch_name)) // TODO remove the dependancy on same branch name and replace with // references stored in .git/ngit }) - .context("cannot find a PR event associated with the checked out branch name")? + .context("cannot find a proposal root event associated with the checked out branch name")? .to_owned(); - println!("found PR event. finding commits..."); + println!("found proposal root event. finding commits..."); let commits_events: Vec = - find_commits_for_pr_event(client, &pr_event, repo_ref).await?; + find_commits_for_proposal_root_event(client, &proposal_root_event, repo_ref).await?; - Ok((pr_event, commits_events)) + Ok((proposal_root_event, commits_events)) } diff --git a/src/sub_commands/send.rs b/src/sub_commands/send.rs index b8cb271..3a9da4b 100644 --- a/src/sub_commands/send.rs +++ b/src/sub_commands/send.rs @@ -140,7 +140,7 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs) -> Result<()> { // oldest first ahead.reverse(); - let events = generate_pr_and_patch_events( + let events = generate_cover_letter_and_patch_events( cover_letter_title_description.clone(), &git_repo, &ahead, @@ -369,7 +369,7 @@ mod tests_unique_and_duplicate { pub static PATCH_KIND: u64 = 1617; -pub fn generate_pr_and_patch_events( +pub fn generate_cover_letter_and_patch_events( cover_letter_title_description: Option<(String, String)>, git_repo: &Repo, commits: &Vec, -- cgit v1.2.3